5
\$\begingroup\$

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:

  1. Tell me if there are any glaring syntactical errors or best practices I've ignored.
  2. 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)
asked Oct 13, 2019 at 23:12
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

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']:
answered Oct 14, 2019 at 19:08
\$\endgroup\$
2
  • 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 and def 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\$ Commented 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\$ Commented Oct 15, 2019 at 19:54

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.