6
\$\begingroup\$

I coded a script to print a 10-day forecast for any location to my terminal.

As I am just learning Python (coming from the PHP world - yes, I know...) and have no buddies coding in Python, I would really appreciate it if anyone could review my code and provide comments and suggestions for improving it.

For starters, I know I am missing docstrings but I left that part for later as I am still reading about it.

Not sure if I need to mention it, but lookup_by_location() and _call() are overriding methods from Weather ancestor class.

Code is also available in a git repo.

#! /usr/bin/env python
import argparse
import requests
from weather import Weather
from weather import models

class WeatherReport(Weather):
    def __init__(self, location):
        Weather.__init__(self)
        self.get_report(location)   

    def get_report(self, location): 
        self.show_location(location)
        [self.show_weather(day) for day in self.get_days(self.lookup_by_location(location))]

    def lookup_by_location(self, location):
        url = "%s?q=select* from weather.forecast " \
              "where woeid in (select woeid from geo.places(1) where text='%s') and u='c' &format=json" % (self.URL, location)
        results = self._call(url)
        return results

    def get_days(self, place):
        days = []
        [self.get_day(days, item['date'], item['low'], item['high'], item['text']) for item in place.forecast()]
        return days

    def get_day(self, days, date, low, high, text):
        days.append([date, int(low), int(high), text])

    def show_weather(self, day):
        from_to = "%d-%d" % (day[1], day[2])
        print " "*2 + "%s:" % (day[0]), from_to.rjust(5) + u'\u00b0' + "C -", "%s" % (day[3])

    def show_location(self, location):
        print "-" * 50
        feedback = "10-day forecast for " + location.capitalize()
        print feedback.center(50)
        print "-" * 50

    def _call(self, url):
        results = requests.get(url).json()
        if int(results['query']['count']) > 0:
            wo = models.weather_obj.WeatherObject(results['query']['results']['channel'])
            return wo
        else:
            print 'No results found.'
            quit()


if __name__ == "__main__":
    parser = argparse.ArgumentParser('tdwreport')
    parser.add_argument('-l', '--location', default="sarajevo", help='get forecast for this location')
    parser.add_argument('-v', '--version', action='version', version='%(prog)s 0.2')
    args = parser.parse_args()
    report = WeatherReport(args.location)
\$\endgroup\$

4 Answers 4

3
\$\begingroup\$

One tip: Instead of gluing constant and variable parts into string with + as e. g. in your statement

print " "*2 + "%s:" % (day[0]), from_to.rjust(5) + u'\u00b0' + "C -", "%s" % (day[3])

use the format() method with placeholders {} in formatted string:

print "  {} {:>5}{}C - {}"     .format(day[0], from_to, u'\u00b0', day[3])

(I visually separated the resulting string with placeholders from the format() method),
or - more descriptively - use names in {} placeholders

print "  {day0} {from_to:>5}{degree_sign}C - {day3}" \
    .format(day0=day[0], from_to=from_to, degree_sign=u'\u00b0', day3=day[3])

Format specifier >5 after : does the same thing as .rjust(5) in your code.


Edit:

For Unicode symbols (as u'\u00b0' in your code) use their hexadecimal value (0x00b0) and type c in the placeholder:

print "  {} {:>5}{:c}C - {}"     .format(day[0], from_to, 0x00b0, day[3])

See Format Specification Mini-Language

\$\endgroup\$
3
  • \$\begingroup\$ Thnx for your reply. I was getting UnicodeEncodeError so I had to modify your code a bit: print " {} {:>5}{}C - {}".format(day[0], from_to, u'\u00b0'.encode('UTF-8'), day[3]) \$\endgroup\$ Commented Jul 3, 2017 at 22:52
  • \$\begingroup\$ Sorry for your inconvenience - I added a more nice approach to the end of my answer. \$\endgroup\$ Commented Jul 3, 2017 at 23:47
  • \$\begingroup\$ To display 0x00b0 in Linux terminal would require some additional changes in environment variables and I would rather avoid that. u'\u00b0'.encode('UTF-8') avoids it. \$\endgroup\$ Commented Jul 4, 2017 at 0:02
2
\$\begingroup\$

Another tip: Your "2-lines returns", e. g.

    results = self._call(url)
    return results

have no advantage over the 1-line ones:

    return self._call(url)
\$\endgroup\$
1
  • \$\begingroup\$ I just copied and pasted this one from ancestor class and modified it to return temperatures in Celsius, but did no refactoring at all. Still a valid point. On the same note, I also stored reference to WeatherReport(args.location) in the report variable in the last line instead of just calling it with WeatherReport(parser.parse_args().location). Thnx again. \$\endgroup\$ Commented Jul 4, 2017 at 0:23
2
\$\begingroup\$

If you swap first 2 lines in your function definition and use longer "---" strings you will reach more impressing visual aspect.

Compare

def show_location(self, location):
    print "-" * 50
    feedback = "10-day forecast for " + location.capitalize()
    print feedback.center(50)
    print "-" * 50

with

def show_location(self, location):
    feedback = "10-day forecast for " + location.capitalize()
    print "-------------------------" * 2
    print feedback.center(50)
    print "-------------------------" * 2
\$\endgroup\$
2
  • \$\begingroup\$ I think this one is a matter of personal preference. But, you gave me an idea: print "-"*50 + "\n", feedback.center(50) + "\n", "-"*50. Is there a way to get this using format()? I am not sure how to repeat string with it. \$\endgroup\$ Commented Jul 4, 2017 at 0:50
  • \$\begingroup\$ Good question - I posted it as a new answer. \$\endgroup\$ Commented Jul 5, 2017 at 17:09
1
\$\begingroup\$

The printing part of

def show_location(self, location):
    print "-" * 50
    feedback = "10-day forecast for " + location.capitalize()
    print feedback.center(50)
    print "-" * 50

may be one line

    print "{0}\n{1:^50}\n{0}".format("-"*50, feedback)

or

    print "{line}\n{fb:^50}\n{line}".format(line="-"*50, fb=feedback)

(It would be a little nicer if you first assign "-"*50 to a variable, say line, and then use it.)


In both cases I used identifications of a placeholder (numbers in the 1st case, names in the 2nd one) to avoid the repetition of "-"*50 - the DRY principle (Don't Repeat Yourself).

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please keep your response together in one answer by editing :) \$\endgroup\$ Commented Jul 5, 2017 at 20:08

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.