13
\$\begingroup\$

I'm looking into basic API examples and I've got the following to work as I want it to. Is there a better way to determine the direction of latitude and longitude? It feels like I could do it better.

import requests
from time import strftime, localtime

response = requests.get('http://api.open-notify.org/iss-now.json')

data = response.json()
status = response.status_code

if status == 200:
    timestamp = data['timestamp']
    time = strftime('%T', localtime(timestamp))

    latitude = data['iss_position']['latitude']
    if float(latitude) > 0:
        lat_dir = 'N'
    else:
        lat_dir = 'S'

    longitude = data['iss_position']['longitude']
    if float(longitude) > 0:
        lon_dir = 'E'
    else:
        lon_dir = 'W'

    print('International Space Station position @ {}'.format(time))
    print('Latitude: {}° {}'.format(latitude, lat_dir))
    print('Longitude: {}° {}'.format(longitude, lon_dir))
else:
    print('Request unsuccessful: {}'.format(status))
\$\endgroup\$
14
\$\begingroup\$

Currently your code is not re-useable. To make it re-useable you should move the logic inside a function.

To do this the function we will have two functions here: _get_data_from_api and get_current_iss_location. Former gets the data from API and latter one passes that data to LatLon class and returns its instance.

The advantage of having two functions here is that the data fetching part and processing part are now isolated and each function is doing a specific thing. Plus having two isolated functions also allows us to test them easily, for example _get_data_from_api function can easily be mocked to return some data in tests and that way our tests won't rely on this external API.

For basic performance improvement we will be using a global session here to re-use previous connections.

Having a separate class LatLon to represent data allows to access the data later on and it can also have other methods that can be used later to do other stuff.

from time import strftime, localtime

import requests


session = requests.Session()


class LatLong:

    def __init__(self, latitude, longitude, timestamp):
        self.latitude = float(latitude)
        self.longitude = float(longitude)
        self.time = strftime('%T', localtime(timestamp))

    @property
    def latitude_hemishere(self):
        return 'N' if self.latitude > 0 else 'S'

    @property
    def longitude_hemisphere(self):
        return 'E' if self.longitude > 0 else 'W'

    def __str__(self):
        return (
            "<Latitude: {self.latitude}° {self.latitude_hemishere} |"
            " Longitude: {self.longitude}° {self.longitude_hemisphere} at {self.time}>").format(self=self)

    __repr__ = __str__

    def pprint(self):
        print('International Space Station position @ {}'.format(self.time))
        print('Latitude: {self.latitude}° {self.latitude_hemishere}'.format(self=self))
        print('Longitude: {self.longitude}° {self.longitude_hemisphere}'.format(self=self))


def _get_data_from_api():
    response = session.get('http://api.open-notify.org/iss-now.json')
    response.raise_for_status()
    return response.json()


def get_current_iss_location():
    data = _get_data_from_api()
    iss_position = data['iss_position']
    return LatLong(iss_position['latitude'], iss_position['longitude'], data['timestamp'])


if __name__ == '__main__':
    get_current_iss_location().pprint()

Now running the module directly will result in:

International Space Station position @ 21:14:51
Latitude: -51.518° S
Longitude: -93.6953° W

We can also import the function in other modules and get the values:

>>> loc = get_current_iss_location()
>>> loc
<Latitude: -51.616° S | Longitude: -89.694° W at 21:15:31>
>>> loc.latitude
-51.616
>>> loc.longitude
-89.694
>>> loc.time
'21:15:31'
>>> loc.pprint()
International Space Station position @ 21:15:31
Latitude: -51.616° S
Longitude: -89.694° W
\$\endgroup\$
  • 1
    \$\begingroup\$ Very good, thank you so much for putting effort into this! I'm only diving into OOP in Python now, I've only done it in Java before, so wasn't entirely sure how to go about doing this in Python. What's the purpose of __repr__ and @property? \$\endgroup\$ – Lukasz Salitra May 30 '17 at 19:43
  • \$\begingroup\$ __repr__ is basically a computer friendly version of an instance, but in this case it doesn't make much sense to have different __str__ and __repr__. repr is also the representation shown by the shell(as shown in the example above). Check: stackoverflow.com/q/1436703/846892 and. property is basically an attribute with custom getter/setter/deleter etc. \$\endgroup\$ – Ashwini Chaudhary May 30 '17 at 20:40
10
\$\begingroup\$

The code is clean and understandable. There are only minor improvements that we can apply.

  • use more descriptive variable names: latitude_direction and longitude_direction instead of lat_dir and lon_dir

  • use ternary conditional operator:

    latitude = data['iss_position']['latitude']
    latitude_direction = 'N' if float(latitude) > 0 else 'S'
    
    longitude = data['iss_position']['longitude']
    longitude_direction = 'E' if float(longitude) > 0 else 'W'
    
  • re-organize imports to follow PEP8 recommendations:

    from time import strftime, localtime
    
    import requests
    
  • you can use f-strings in case of Python 3.6+:

    print(f'International Space Station position @ {time}')
    print(f'Latitude: {latitude}° {latitude_direction}')
    print(f'Longitude: {longitude}° {longitude_direction}')
    

There is also that handy geopy package. It is, probably, an overkill to use it just for this simple problem, but, if you are going to do more complex geocoding things, check it out.

\$\endgroup\$
  • \$\begingroup\$ Thanks so much! I can tell the conditional operators are gonna make me very happy, so much cleaner! :D \$\endgroup\$ – Lukasz Salitra May 30 '17 at 19:42

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.