This code represents my first real Python 3 program. It retrieves flood data from the NWS weather center for the river near my home and posts a warning to my Facebook page whenever certain flood levels have been met. To me the script just looks bulky and not very elegant. The script returns results fairly quickly but it seems like the same results could be produced with less work, or less list-building, as it were. Is this possible without using so many lists, or in a more Pythonic way?

#!/usr/bin/env python3

Facebook Flood Warning Alert System - this script will post a notification to
to Facebook whenever the Sabine River triggers a stage warning.

import requests
import facebook
from bs4 import BeautifulSoup as BS

with open ('fbkey', 'r') as f:
    key = f.readlines()

graph = facebook.GraphAPI(access_token = key) 

river_url = 'http://water.weather.gov/ahps2/river.php?wfo=SHV&wfoid=18715&riverid=203413&pt%5B%5D=all&allpoints=143204%2C147710%2C141425%2C144668%2C141750%2C141658%2C141942%2C143491%2C144810%2C143165%2C145368&data%5B%5D=xml'

headers = {"user-agent" : "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0"}
r= requests.get(river_url, headers=headers)
soup = BS(r.text, 'lxml')

#process the data returned from waterdata.usgs.gov

stations = soup.find_all('h1', attrs={'class': 'data_name'})
stages = soup.find_all('div', attrs={'class': 'stage_stage_flow'})
flood_lvl = soup.find_all('div', attrs={'class': 'flood_stage_flow'})
warns = soup.find_all('div', attrs={'class': 'current_warns_statmnts_ads'})
alerts = soup.find_all('td', attrs={'scope': 'col'})

num_river = len(stations)
#strip the html formatting and build our lists
stx_list = []
stage_list = []
flood_list = []
warns_list = []

for i in range(num_river):

#setup the alert system with appropriate values and corresponding warning stage for each river station.
alert_list = []
alert_values = []
for i in range(len(alerts)):

a_values = alert_list[1::2]

#get current river level (stage)
stages = []
levels = []
for i in range(num_river):
    s = stage_list[i].split()
for i in range(num_river):
    s = stages[i][2]


#build current warning list
warns = []
for i in range(len(warns_list)):
    w = warns_list[0].strip('Current Warnings/Statements/Advisories:')

#build Major, Moderate, Flood, Action values lists
major_lvl = a_values[::5]
moderate_lvl = a_values[1::5]
flood_lvl = a_values[2::5]
action_lvl = a_values[3::5]

num_stx = len(stx_list)
class River():
   def __init__(self, station, stage, major, moderate, flood, action, warn):
       self.station = station
       self.stage = stage
       self.major = major
       self.moderate = moderate
       self.flood = flood
       self.action = action
       self.warn = warn

       if self.major == 0:
            self.major = self.action
       if self.moderate == 0:
            self.moderate = self.action
       if self.flood == 0:
            self.flood = self.action

   def alerts(self):          
        if float(self.stage) < float(self.action):
        elif float(self.stage) >= float(self.major):
            graph.put_object(parent_object='me', connection_name='feed', message=('The %s has reached [Major Flood Stage: (%sFt)] @ %sFt.\n\n***Warnings***\n%s\n\nPlease click the Link below for more information.' % (self.station, self.major, self.stage, self.warn)), link = river_url)
        elif float(self.stage) >= float(self.moderate):
            maj_diff = round(float(self.stage) - float(self.moderate), 2)
            graph.put_object(parent_object='me', connection_name='feed', message=('The %s has reached [Moderate Flood Stage: (%sFt)] @ %sFt.\n\nNext stage is [Major Flood Stage] in %sFt.\n\n***Warnings***\n%s\n\nPlease click the Link below for more information.' % (self.station, self.moderate, self.stage, maj,diff, self.warn)), link = river_url)
        elif float(self.stage) >= float(self.flood):
            mod_diff = round(float(self.stage) - float(self.flood), 2)
            warn = graph.put_object(parent_object='me', connection_name='feed', message=('The %s has reached [Flood Stage: (%sFt)] @ %sFt.\n\nNext stage is [Moderate Flood Stage] in %sFt.\n\n***Warnings***\n%s\n\nPlease click the Link below for more information.' % (self.station, self.flood, self.stage, mod_diff, self.warn)), link = river_url)
        elif float(self.stage) >= float(self.action):
            flood_diff = round(float(self.stage) - float(self.action), 2)
            warn = graph.put_object(parent_object='me', connection_name='feed', message=('The %s has reached [Action Flood Stage: (%sFt)] @ %sFt.\n\nNext stage is [Flood Stage] in %sFt.\n\n***Warnings***\n%s\n\nPlease click the Link below for more information.' % (self.station, self.action, self.stage, flood_diff, self.warn)), link = river_url)

def riverlist():
    river_list = []
    for n in range(len(stx_list)):
        station = River(stx_list[n], levels[n], major_lvl[n], moderate_lvl[n], flood_lvl[n], action_lvl[n], warns[n])
    return river_list

if __name__ == '__main__':
    for river in riverlist():

Improving Data-mining

Using a list of dictionaries

What if you, instead of keeping and supporting multiple data structures, would construct a list of dictionaries where each dictionary correspond to a river from the National Weather Service page.

The idea is to loop over the river name header elements, find the next div elements which contain "river" specific data; locate and collect the desired flood level and warnings data. For the alerts, we can pre-configure the list of alert levels and "zip" them with the actual alert level values for every river.

Here is the improved code:

from pprint import pprint

import requests
from bs4 import BeautifulSoup as BS

ALERT_LEVELS = ["major", "moderate", "flood", "action", "low"]
RIVER_URL = 'http://water.weather.gov/ahps2/river.php?wfo=SHV&wfoid=18715&riverid=203413&pt%5B%5D=all&allpoints=143204%2C147710%2C141425%2C144668%2C141750%2C141658%2C141942%2C143491%2C144810%2C143165%2C145368&data%5B%5D=xml'

headers = {"user-agent": "Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0"}
response = requests.get(RIVER_URL, headers=headers)
soup = BS(response.text, 'lxml')

data = []
# process the data returned from waterdata.usgs.gov
for river in soup.select("h1.data_name"):
    river_name = river.get_text(strip=True)

    river_data = river.find_next_sibling("div")

        "name": river_name,
        "stage": river_data.select_one(".stage_stage_flow").get_text(strip=True).replace("Latest Stage: ", ""),
        "flood_lvl": river_data.select_one(".flood_stage_flow").get_text(strip=True).replace("Flood Stage: ", "").replace(" Feet", ""),
        "warns": river_data.select_one(".current_warns_statmnts_ads > b").next_sibling.strip(),
        "alerts": {
            alert_name: alert_value.get_text(strip=True)
            for alert_name, alert_value in
        zip(ALERT_LEVELS, river_data.select(".flood_his_lwr .box_square table tr > td:nth-of-type(2)"))


pprint() here is just to pretty-print the resulting data-structure. Here is what it would output:

[{'alerts': {'action': '13.5',
             'flood': '14',
             'low': '0',
             'major': '21',
             'moderate': '18'},
  'flood_lvl': '14',
  'name': 'Sabine River Near Mineola (MLAT2)',
  'stage': '13.75',
  'warns': 'None currently.'},
 {'alerts': {'action': '22.5',
             'flood': '23',
             'low': '0',
             'major': '37',
             'moderate': '32'},
  'flood_lvl': '23',
  'name': 'Sabine River Near Hawkins (HAKT2)',
  'stage': '13.67',
  'warns': 'None currently.'},
 {'alerts': {'action': '25',
             'flood': '26',
             'low': '0',
             'major': '36',
             'moderate': '33'},
  'flood_lvl': '26',
  'name': 'Sabine River Near Gladewater (GDWT2)',
  'stage': '16.86',
  'warns': 'None currently.'},
 {'alerts': {'action': '23',
             'flood': '25',
             'low': '0',
             'major': '35',
             'moderate': '30'},
  'flood_lvl': '25',
  'name': 'Sabine River Below Longview (LONT2)',
  'stage': '16.96',
  'warns': 'None currently.'},
 {'alerts': {'action': '25.5',
             'flood': '26',
             'low': '0',
             'major': '35',
             'moderate': '30'},
  'flood_lvl': '26',
  'name': 'Sabine River Near Beckville (BEKT2)',
  'stage': '18.81',
  'warns': 'None currently.'},
 {'alerts': {'action': '27',
             'flood': '28',
             'low': '0',
             'major': '34',
             'moderate': '32'},
  'flood_lvl': '28',
  'name': 'Sabine River At Logansport (LPTL1)',
  'stage': '24.22',
  'warns': 'None currently.'},
 {'alerts': {'action': '172.5',
             'flood': '0',
             'low': '165',
             'major': '0',
             'moderate': '0'},
  'flood_lvl': 'Not available',
  'name': 'Sabine River At Toledo Bend Reservoir (BKLT2)',
  'stage': '172.28',
  'warns': 'None currently.'},
 {'alerts': {'action': '38',
             'flood': '43',
             'low': '0',
             'major': '47',
             'moderate': '45'},
  'flood_lvl': '43',
  'name': 'Sabine River Near Burkeville (BRVT2)',
  'stage': '24.23',
  'warns': 'None currently.'},
 {'alerts': {'action': '30',
             'flood': '30',
             'low': '-9999',
             'major': '36',
             'moderate': '33'},
  'flood_lvl': '30',
  'name': 'Sabine River Near Bon Wier (BWRT2)',
  'stage': '22.83',
  'warns': 'None currently.'},
 {'alerts': {'action': '23',
             'flood': '24',
             'low': '0',
             'major': '28',
             'moderate': '26'},
  'flood_lvl': '24',
  'name': 'Sabine River At Deweyville (DWYT2)',
  'stage': '24.22',
  'warns': 'None currently.'},
 {'alerts': {'action': '3.5',
             'flood': '4',
             'low': '0',
             'major': '6',
             'moderate': '5'},
  'flood_lvl': '4',
  'name': 'Sabine River At Orange (ORNT2)',
  'stage': '2.31',
  'warns': 'None currently.'}]

Code Style and Code Organization Notes

  • you should wrap the data-mining/web-scraping part of the code into it's own function and call inside the if __name__ == '__main__': to avoid executing the code on import and to make the code reusable
  • I don't think you really need a class for this kind of a isolated problem - especially, if you are going to follow my advice to keep the data in the list of dictionaries. I would simply post-process the resulting data in a function without instantiating extra River classes. Related blogpost: Stop Writing More Classes
  • it would probably be a good idea to improve the code I've posted above and convert strings to floats to have this data structure contain everything needed for analyzing and reporting
  • 2
    \$\begingroup\$ First let me say thanks for the tips! To be honest the way your solution looks is exactly how I picture the process in my head, but couldn't translate that into the code, most likely due to my inexperience with the syntax and functions. I really like this method because as you said it is processing the station data per station rather than iterating through a list and hoping the elements line up. In the future I would like to apply this to all rivers, and your method seems like it would accommodate this. I'll implement your solution and see how it works with multiple rivers. \$\endgroup\$ Jun 6 '17 at 15:28
  • \$\begingroup\$ Question: when I'm ready to add updates to my code should I do so in the OP as an Edit or as an Answer, or start a new thread entirely? I'm new to code review and while I've read thru the FAQs I'm still unsure of the correct etiquettes here. \$\endgroup\$ Jun 8 '17 at 15:57
  • 1
    \$\begingroup\$ @AaronNelson definitely a new question - it is perfectly okay to go through multiple rounds of code reviews. Thanks. \$\endgroup\$
    – alecxe
    Jun 8 '17 at 15:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.