10
\$\begingroup\$

I'm new to programming. I wrote this following weather app in python. This app can provide simple weather forecast for three days. If the user provides the location in the correct format then it should be able to give weather information. I hope someone could point me if there is any mistake in the code or make it better.

import requests as r
from bs4 import BeautifulSoup as BS
import datetime
import pickle
import os


CURRENT_LOCATION = ''


def weather_server_one(location):
    """ Server_one scrapping algorithm to get weather information """

    net_report_cel = ''
    l = ''
    m = []

    source_code = r.get(location)
    plain_text = source_code.text
    soup = BS(plain_text, 'html.parser')

    for tag in soup.find_all('span', {'class': 'warm txt-xxlarge'}):
        for i in tag.find_all('strong'):
            net_report_cel += i.string + '° C'

    for tag2 in soup.find_all('div', {'class': 'c2_a'}):
        for tx in tag2.find_all('span'):
            for k in tx.find_all('strong'):
                l += k.string + ','

    for tag3 in soup.find_all('div', {'class': 'c2_a'}):
        for max in tag3.find_all('a'):
            title = max.get('title')
            m.append(title)

    a, b, c = [x for x in m]
    x = (a, b, c)

    day1_h, day1_l, day2_h, day2_l, day3_h, day3_l = l.rstrip(',').split(',')
    return day1_h, day1_l, day2_h, day2_l, day3_h, day3_l, net_report_cel, x


def weekday():
    """ A function that returns the name of today, tomorrow, and day after tomorrow """
    days = {0: 'Monday',
        1: 'Tuesday',
        2: 'Wednesday',
        3: 'Thursday',
        4: 'Friday',
        5: 'Saturday',
        6: 'Sunday'
        }

    x = datetime.datetime.today().weekday()

    if x in days:
        if x == 5:  # In case of Saturday
            today, tomo, day_a_to = days[x], days[x + 1], days[0]
            return today, tomo, day_a_to
        elif x == 6:  # In case of Sunday
            today, tomo, day_a_to = days[x], days[0], days[1]
            return today, tomo, day_a_to
        else:  # All other weekdays
            today, tomo, day_a_to = days[x], days[x + 1], days[x + 2]
            return today, tomo, day_a_to


def reloop_location_entry():
    """ A function that returns the URL for the server_one to get information from """

    print('Please provide the location in following format:\n'
      'Stockholm, Sweden, Europe')
    x = input('>>> ')
    a, b, c = x.split(', ')
    loc = 'http://www.foreca.com/%s/%s/%s' % (c, b, a)
    global CURRENT_LOCATION
    CURRENT_LOCATION += loc
    return loc


def location_grabber():
    """ A function that returns the stored weather location, and in case of failure it returns new weather location """

    try:
        x = save_load_cu_lo(load=True)
        global CURRENT_LOCATION
        CURRENT_LOCATION += str(x)
        return str(x)
    except FileNotFoundError:
        return reloop_location_entry()


def display_report(try_=False):
    """ This function prints out the scraped weather information into a desired pattern """

    if try_ is False:
        l = location_grabber()
    else:
        l = reloop_location_entry()
    today, tomorrow, day_after_tomo = weekday()
    tod_hi, tod_low, tomo_hi, tomo_low, day_a_to_hi, day_a_to_low, current_weather, plain_report = weather_server_one(l)
    to_report, tomo_report, day_a_to_report = plain_report
    print('Today\'s(%s) weather report suggests: %s\n' % (today, to_report.capitalize()))
    print('Today current temperature is %s' % current_weather)
    print('      maximum temperature is %s C\n''      minimum temperature is %s C\n' % (tod_hi, tod_low))
    print('Tomorrow\'s(%s) weather report suggests: %s\n' % (tomorrow, tomo_report.capitalize()))
    print('Tomorrow maximum temperature is %s C\n''         minimum temperature is %s C\n' % (tomo_hi, tod_low))
    print('%s\'s weather report suggests: %s\n' % (day_after_tomo, day_a_to_report.capitalize()))
    print('%s maximum temperature is %s C\n''and minimum temperature is %s C\n' % (day_after_tomo, tod_hi, tod_low))


def save_load_cu_lo(load=False):
    """ It allows to store or load weather location/URL to/from a stored file in HDD """

    global CURRENT_LOCATION
    file_path = os.path.join(os.path.expanduser('~'), 'Desktop', str(os.path.basename(__file__) + '_data.p'))
    if load is False:
        pickle.dump(CURRENT_LOCATION, (open(file_path, 'wb')))
    else:
        x = pickle.load(open(file_path, 'rb'))
        return x


def try_again():
    """ Part of repeat() function, basically obtains user input and returns it """

    while True:
        try:
            x = int(input('Press 1 to add new location\n'
                      'Press 2 to reload the current location\n'
                      'Press 3 to quit\n'))
            if x == 1 or x == 2 or x == 3:
                return x
            print('Either press 1 or 2 or 3')
        except ValueError:
            print('Select an integer')


def clear_cu():
    """ It deletes CURRENT_LOCATION variable from the memory and creates an empty CURRENT_LOCATION variable """

    global CURRENT_LOCATION
    del CURRENT_LOCATION
    globals()['CURRENT_LOCATION'] = ''


def repeat():
    """ This allows the process of repeating the act of server_one or quit from the app """

    x = try_again()
    if x == 1:
        display_report(try_=True)  # WHen asking for new location
        save_load_cu_lo()
        clear_cu()
    elif x == 2:
        display_report()  # WHen reloading the same location
        save_load_cu_lo()
        clear_cu()
    else:
        quit()


def main():
    print('Welcome to WeatherPy by AJ')
    display_report()  # Initially for the new location
    save_load_cu_lo()
    clear_cu()
    while True:
        repeat()


if __name__ == '__main__':
    main()
\$\endgroup\$
7
\$\begingroup\$
  • One of the big issue with this program is the variable names. Variable names like a, b, c, x, l(l looks like 1(one) too) etc barely make any sense and you yourself won't be able to know what they represent in couple of months time. Hence, always go for meaningful variable names. For example a, b, c = x.split(', ') in reloop_location_entry() can easily be replaced with city, country, continent = user_input.split(', ').

  • Checks like if x == 1 or x == 2 or x == 3 can be reduced to if x in {1, 2, 3}.

  • Start using standard library, it will reduce some of your code. The function weekday() which should really be named get_today_and_next_two_days can use the calendar module.

    def get_today_and_next_n_days(n_days):
        """ A function that returns the name of today and next n days"""
        weekday = datetime.datetime.today().weekday()
        days = list(calendar.day_name)
        return [days[n % len(days)] for n in range(weekday, weekday + n_days + 1)]
    
  • Use try-except only around the statements where you expect the exception not around a bunch of unrelated statements For example location_grabber() can be re-written as:

    try:
        location_url = save_load_cu_lo(load=True)
    except FileNotFoundError:
        return reloop_location_entry()
    else:
        global CURRENT_LOCATION
        CURRENT_LOCATION += location_url
        return location_url
    
  • Always open the files using with-statement.

    It is good practice to use the with keyword when dealing with file objects. The advantage is that the file is properly closed after its suite finishes, even if an exception is raised at some point. Using with is also much shorter than writing equivalent try-finally blocks.

  • Group your imports as mentioned in PEP-8. Imports should be grouped in the following order(you should put a blank line between each group of imports):

    1. standard library imports
    2. related third party imports
    3. local application/library specific imports

\$\endgroup\$
  • \$\begingroup\$ You were right, I wouldn't remember such variable names in a month, I was being lazy. I understand standard libs are for our rescues still I would prefer to learn the alternative routes as well. Finally, thank you so much for your comments, they are really useful for me as a beginner to the programming world. \$\endgroup\$ – A J Aug 20 '17 at 15:29
4
\$\begingroup\$

Aside from the very good points described by Ashwini, you can also improve the HTML parsing part and simplify the ways you are extracting the texts and attributes of the elements by switching to CSS selectors and using str.join().

Replace:

for tag in soup.find_all('span', {'class': 'warm txt-xxlarge'}):
    for i in tag.find_all('strong'):
        net_report_cel += i.string + '° C'

with:

net_report_cel = soup.select_one(".txt-xxlarge strong").get_text(strip=True) + '° C'

Replace:

for tag2 in soup.find_all('div', {'class': 'c2_a'}):
    for tx in tag2.find_all('span'):
        for k in tx.find_all('strong'):
            l += k.string + ','

with:

l = ",".join([item.get_text(strip=True) 
              for item in soup.select(".c2_a span strong")])
\$\endgroup\$
  • \$\begingroup\$ Thank you so much for these useful shortcuts, I'm very new to programming. I need to learn a lot from you guys. Btw, @alecxe you seem to be a master of Beautifulsoup, could you suggest me some good books or online resources? Thanks again. \$\endgroup\$ – A J Aug 20 '17 at 15:33
  • \$\begingroup\$ @AJ thanks for the kind words. BS4's documentation is a good place to start. Also, study popular BS4 questions on SO..and, I guess, practice and practice :) \$\endgroup\$ – alecxe Aug 20 '17 at 16:29

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.