This script sends and receives text messages and logs responses in a google sheet. Demo Gif here: https://github.com/RyGuy96/HealthTracker.
I'm a self-taught beginner coder just starting my formal education in college. I've never had an experienced programmer look over my work; it was suggested to me that I needed to start building my professional profile. I'd be very grateful if someone would look through this script and:
- Tell me if there are any glaring syntactical errors or best practices I've ignored.
- Suggest any obvious ways it might be improved in terms of readability or speed.
Thank you, I really do appreciate any and all advice! This is my first post here - if there something I've missed in the guidelines please let me know.
#!/usr/bin/env python
"""receiver.py: Receives an sms message to a specifind number; parses and saves data in a Google Spreadsheet."""
from flask import Flask, request
from twilio.twiml.messaging_response import MessagingResponse
import gspread
import re
import datetime
import time
from twilio.rest import Client
from oauth2client.service_account import ServiceAccountCredentials
import pytz
def sms_sender(message_text: str, recipient_num: str) -> None:
"""Define credentials for Twilio API sms."""
# Find these values at https://twilio.com/user/account (note that this is not a secure method)
account_sid = "YOUR_SID"
auth_token = "YOUR_AUTH_TOKEN"
client = Client(account_sid, auth_token)
message = client.api.account.messages.create(to= recipient_num,
from_="YOUR_TWILIO_NUMBER",
body= message_text)
def open_spreadsheet() -> object:
"""Authorize Google Drive access and return spreadsheet reference."""
scope = ['https://spreadsheets.google.com/feeds', 'https://www.googleapis.com/auth/drive']
credentials = ServiceAccountCredentials.from_json_keyfile_name('JSON_FILE', scope)
gc = gspread.authorize(credentials)
wks_health = gc.open("Health Tracker").sheet1
return wks_health
def get_date() -> datetime:
utc_now = pytz.utc.localize(datetime.datetime.utcnow())
return datetime.datetime.strftime(utc_now.astimezone(pytz.timezone("America/Chicago")), '%Y-%m-%d')
def parse_sms(sms_reply: str) -> dict:
"""Return single dictionary of relevant values in sms."""
# ratings
ratings = re.compile(r"\b\d+\b")
rating_vals = (ratings.findall(sms_reply)[:5])
# added meds
add = re.compile(r"\+(.*?\))")
add_meds = add.findall(sms_reply)
# removed meds
remove = re.compile(r"\-(.*?\))")
remove_meds = remove.findall(sms_reply)
# notes
note = re.compile(r"Note\((.*?)\)", flags=re.IGNORECASE)
add_note = [] if not note.findall(sms_reply) else note.findall(sms_reply)[0]
# help
helpme_variants = ["help me", "helpme", "help-me"]
display_help = any([re.findall(i, sms_reply, flags=re.IGNORECASE) for i in helpme_variants])
# see meds
med_variants = ["seeMeds", "see-meds", "see meds"]
display_meds = any([re.findall(i, sms_reply, flags=re.IGNORECASE) for i in med_variants])
parsed_response = {"ratings": rating_vals, "add meds": add_meds, "remove meds": remove_meds, "notes": add_note,
"display help": display_help, "display meds": display_meds}
return parsed_response
def get_current_meds() -> str:
wks_health = open_spreadsheet()
time.sleep(4)
return wks_health.acell('G2').value.strip('][').split(', ')
def validate_sms(parsed_response: dict) -> str:
"""Check sms and return 'Valid' or one or more error messages to be sent to user."""
invalid_responses = []
try:
assert len(parsed_response["ratings"]) == 5
except AssertionError:
invalid_responses.append("Invalid number of ratings (there should be five)")
current_meds = get_current_meds()
try:
for med in parsed_response["remove meds"]:
assert med in current_meds
except AssertionError:
invalid_responses.append("Med to remove not found, see your meds by replying 'see-meds'")
try:
for med in parsed_response["add meds"]:
assert med not in current_meds
except AssertionError:
invalid_responses.append("Med to be added already listed, see your meds by replying 'see-meds'")
finally:
if invalid_responses:
return ", ".join(invalid_responses)
else:
return "Valid"
def record_sms(parsed_response: dict) -> None:
"""Log sms responses in Google Sheets."""
note = parsed_response["notes"]
remove = parsed_response["remove meds"]
add = parsed_response["add meds"]
current_meds = get_current_meds()
revised_med_list = [med for med in current_meds if med not in remove] + add
revised_med_list_formated = str(revised_med_list).replace("\'", "")
line = [get_date()] + parsed_response["ratings"] + [revised_med_list_formated]
line.append(note if note else "")
wks_health = open_spreadsheet()
wks_health.insert_row(line, value_input_option='USER_ENTERED', index=2)
def help_message() -> str:
# change symptoms as you see fit - some refactoring will be required if you you want to track more or less than five
message = "Respond to messages with: " \
"\n1. Hours slept " \
"\n2. Stress level (1-9) " \
"\n3. Joints (1-9) " \
"\n4. Energy (1-9) " \
"\n5. Mood (1-9) " \
"\n6. Add a note with NOTE(YOUR NOTE)* " \
"\n7. Add a med with +MEDNAME(DOSE)* " \
"\n8. Remove a med with -MEDNAME(DOSE)* " \
"\n9. See all meds with 'see-meds'* " \
"\n10. See this menu with 'help-me'*" \
"\n*Optional values in response"
return message
def see_meds_message() -> str:
message = "Your current meds are: " + str(get_current_meds())
return message
app = Flask(__name__)
@app.route("/sms", methods=['GET', 'POST'])
def main() -> str:
"""Listen for incoming sms and log content or reply with one or more error messages."""
from_body = request.values.get('Body', None)
resp = []
while from_body is not None:
try:
sms = parse_sms(from_body)
except:
resp.append("issue parsing")
break
try:
if sms['display help'] == True:
resp.append(help_message())
break
except:
resp.append("issue with help message")
break
try:
if sms['display meds'] == True:
resp.append(see_meds_message())
break
except:
resp.append("issue with showing meds")
break
try:
validation_val = validate_sms(sms)
except:
resp.append("issue validating")
break
try:
if validation_val == "Valid":
record_sms(sms)
resp.append("Response recorded!")
break
except:
resp.append("issue logging valid sms")
break
try:
if validation_val != "Valid":
resp.append(validation_val)
break
except:
resp.append("issue logging invalid sms")
break
mess= MessagingResponse()
mess.message(str(", ".join(resp)))
return str(mess)
if __name__ == "__main__":
app.run(debug=True)
1 Answer 1
Regex compilation
Your code here:
# ratings
ratings = re.compile(r"\b\d+\b")
rating_vals = (ratings.findall(sms_reply)[:5])
# added meds
add = re.compile(r"\+(.*?\))")
add_meds = add.findall(sms_reply)
# removed meds
remove = re.compile(r"\-(.*?\))")
remove_meds = remove.findall(sms_reply)
gets only halfway to a good idea. It is useful to separate regex compilation from regex execution, but only if the compiled regexes are persisted. You can put them as constants in global scope. Then the compilation cost is only paid once.
Help me
helpme_variants = ["help me", "helpme", "help-me"]
display_help = any([re.findall(i, sms_reply, flags=re.IGNORECASE) for i in helpme_variants])
First of all: any
doesn't require a list; you should be passing the generator to any
directly.
Beyond that: you don't need "variants" or a generator at all; you can do this with another regex:
display_help_re = re.compile(r'help[ -]?me', flags=re.IGNORECASE)
# ...
display_help = display_help_re.search(sms_reply) is not None
The same is true of your med_variants
.
Waiting for...?
time.sleep(4)
Why is this here? It's spooky. At the very least, drop a comment. More likely is that you should attempt some manner of polling if possible to detect when the given condition is met.
Magic values
'G2'
is a magic value. At the least, it should be put into a named constant. More likely is that you should be making a named range in your spreadsheet and using that range, if this is at all possible.
5 is also a magic value here:
assert len(parsed_response["ratings"]) == 5
Logic by exception
This:
try:
for med in parsed_response["remove meds"]:
assert med in current_meds
except AssertionError:
invalid_responses.append("Med to remove not found, see your meds by replying 'see-meds'")
is awkward and not necessary. Just do the check yourself, and use a set
instead:
parsed_meds = set(parsed_response['remove_meds'])
for missing_med in parsed_meds - current_meds:
invalid_responses.append(f'Med to remove "{missing_med} not found; see your meds by replying "see-meds"')
Character escapes
"\'"
does not need an escape backslash.
String continuation
Rather than using backslashes here:
message = "Respond to messages with: " \
"\n1. Hours slept " \
"\n2. Stress level (1-9) " \
"\n3. Joints (1-9) " \
"\n4. Energy (1-9) " \
"\n5. Mood (1-9) " \
"\n6. Add a note with NOTE(YOUR NOTE)* " \
"\n7. Add a med with +MEDNAME(DOSE)* " \
"\n8. Remove a med with -MEDNAME(DOSE)* " \
"\n9. See all meds with 'see-meds'* " \
"\n10. See this menu with 'help-me'*" \
"\n*Optional values in response"
Put the whole thing in parens and drop the backslashes.
f-strings
def see_meds_message() -> str:
message = "Your current meds are: " + str(get_current_meds())
return message
can be
def see_meds_message() -> str:
return f'Your current meds are: {get_current_meds()}'
Implicit None
Drop the , None
from this:
from_body = request.values.get('Body', None)
Because that's already the default for get
.
Boolean comparison
if sms['display help'] == True:
should simply be
if sms['display help']:
-
1\$\begingroup\$ You are a wizard, thank you so much Reinderien. The "magic" values 5 and G2 are used in multiple functions. I assume you would declare a function that returns those Constants? e.g.
def NUM_OF_RATINGS(): return 5
anddef MEDS_CELL(): return chr(ord('a') + 1 + NUM_OF_RATINGS()) + "2"
? Would you group both the constant functions as methods in a constant class? \$\endgroup\$RyGuy– RyGuy2019年10月15日 17:52:34 +00:00Commented Oct 15, 2019 at 17:52 -
1\$\begingroup\$ Nah; it's easier just to declare a globally-scoped constant variable,
NUM_OF_RATINGS = 5
. \$\endgroup\$Reinderien– Reinderien2019年10月15日 19:54:29 +00:00Commented Oct 15, 2019 at 19:54