Some pylint-inspired clean-up. Also added pylint test to Travis.

Author: oschwald

.travis.yml

@@ -5,11 +5,14 @@ python:
   - 3.2
   - 3.3
   - pypy
-script: nosetests
 install:
   - pip install -r requirements.txt
   - pip install git+https://github.com/maxmind/HTTPretty.git
+  - pip install pylint
   - if [[ $TRAVIS_PYTHON_VERSION == '2.6' ]]; then pip install unittest2; fi
+script:
+  - nosetests
+  - pylint geoip2
 notifications:
   email:
     recipients:

geoip2/__init__.py

@@ -1,3 +1,8 @@
+"""
+=================
+GeoIP2 Python API
+=================
+"""
 __title__ = 'geoip2'
 __version__ = '0.1'
 __author__ = 'Gregory Oschwald'

geoip2/errors.py

@@ -14,7 +14,7 @@ class GeoIP2Error(RuntimeError):
     """
 
 
-class GeoIP2HTTPError(GeoIP2Error):
+class HTTPError(GeoIP2Error):
     """There was an error when making your HTTP request.
 
     This class represents an HTTP transport error. It extends
@@ -25,22 +25,22 @@ class GeoIP2HTTPError(GeoIP2Error):
 
     """
     def __init__(self, message, http_status=None, uri=None):
-        super(GeoIP2HTTPError, self).__init__(message)
+        super(HTTPError, self).__init__(message)
         self.http_status = http_status
         self.uri = uri
 
 
-class GeoIP2WebServiceError(GeoIP2HTTPError):
+class WebServiceError(HTTPError):
     """The GeoIP2 web service returned an error.
 
     This class represents an error returned by MaxMind's GeoIP2 Precision
-    web service. It extends :py:exc:`GeoIP2HTTPError`.
+    web service. It extends :py:exc:`HTTPError`.
 
     :ivar code: The code returned by the MaxMind web service
     :ivar http_status: The HTTP status code returned
     :ivar uri: The URI queried
 
     """
     def __init__(self, message, code=None, http_status=None, uri=None):
-        super(GeoIP2WebServiceError, self).__init__(message, http_status, uri)
+        super(WebServiceError, self).__init__(message, http_status, uri)
         self.code = code

geoip2/models.py

@@ -10,6 +10,7 @@
 http://dev.maxmind.com/geoip/geoip2/web-services for more details.
 
 """
+#pylint:disable=R0903
 import geoip2.records
 
 
@@ -50,10 +51,10 @@ def __init__(self, raw_response, languages=None):
             geoip2.records.Country(languages,
                                    **raw_response.get('registered_country',
                                                       {}))
-        self.represented_country = \
-            geoip2.records.RepresentedCountry(languages,
-                                   **raw_response.get('represented_country',
-                                                      {}))
+        self.represented_country \
+            = geoip2.records.RepresentedCountry(languages,
+                                                **raw_response.get(
+                                                'represented_country', {}))
 
         self.traits = geoip2.records.Traits(**raw_response.get('traits', {}))
         self.raw = raw_response

geoip2/records.py

@@ -4,21 +4,22 @@
 =======
 
 """
-
+#pylint:disable=R0903
 from abc import ABCMeta
-import collections
 
 
 class Record(object):
     """All records are subclasses of ``Record``"""
     __metaclass__ = ABCMeta
 
+    _valid_attributes = None
+
     def __init__(self, **kwargs):
         valid_args = dict((k, kwargs.get(k)) for k in self._valid_attributes)
         self.__dict__.update(valid_args)
 
     def __setattr__(self, name, value):
-        raise NotImplementedError(name + ' is read-only.')
+        raise AttributeError("can't set attribute")
 
 
 class PlaceRecord(Record):
@@ -35,6 +36,8 @@ def __init__(self, languages=None, **kwargs):
 
     @property
     def name(self):
+        """Dict with language codes as keys and localized name as value"""
+        #pylint:disable=E1101
         return next((self.names.get(x) for x in self._languages if x in
                      self.names), None)
 
@@ -100,9 +103,9 @@ class Country(PlaceRecord):
       end point.
     :ivar geoname_id: The GeoName ID for the country. This attribute is
       returned by all end points.
-    :ivar iso_code: The two-character ISO 3166-1
-      (http://en.wikipedia.org/wiki/ISO_3166-1) alpha code for the country.
-      This attribute is returned by all end points.
+    :ivar iso_code: The two-character `ISO 3166-1
+      <http://en.wikipedia.org/wiki/ISO_3166-1>`_ alpha code for the
+      country. This attribute is returned by all end points.
     :ivar name: The name of the country based on the languages list
       passed to the constructor. This attribute is returned by all end points.
     :ivar names: A dictionary where the keys are language codes and the values
@@ -115,8 +118,8 @@ class Country(PlaceRecord):
 class RepresentedCountry(Country):
     """Contains data for the represented country associated with an IP address
 
-    This class contains the country-level data associated with an IP address for
-    the IP's represented country. The represented country is the country
+    This class contains the country-level data associated with an IP address
+    for the IP's represented country. The represented country is the country
     represented by something like a military base or embassy.
 
     This record is returned by all the end points.
@@ -128,8 +131,8 @@ class RepresentedCountry(Country):
       end point.
     :ivar geoname_id: The GeoName ID for the country. This attribute is
       returned by all end points.
-    :ivar iso_code: The two-character ISO 3166-1
-      (http://en.wikipedia.org/wiki/ISO_3166-1) alpha code for the country.
+    :ivar iso_code: The two-character `ISO 3166-1
+      <http://en.wikipedia.org/wiki/ISO_3166-1>`_ alpha code for the country.
       This attribute is returned by all end points.
     :ivar name: The name of the country based on the languages list
       passed to the constructor. This attribute is returned by all end points.
@@ -141,7 +144,8 @@ class RepresentedCountry(Country):
       endpoints.
 
     """
-    _valid_attributes = set(['confidence', 'geoname_id', 'iso_code', 'names', 'type'])
+    _valid_attributes = set(['confidence', 'geoname_id', 'iso_code',
+                             'names', 'type'])
 
 
 class Location(Record):
@@ -164,13 +168,13 @@ class Location(Record):
       except the Country end point.
     :ivar metro_code: The metro code of the location if the
       location is in the US. MaxMind returns the same metro codes as the
-      Google AdWords API
-      (https://developers.google.com/adwords/api/docs/appendix/cities-DMAregions).
+      `Google AdWords API
+      <https://developers.google.com/adwords/api/docs/appendix/cities-DMAregions>`_.
       This attribute is returned by all end points except the Country end
       point.
     :ivar time_zone: The time zone associated with location, as
-      specified by the IANA Time Zone Database
-      (http://www.iana.org/time-zones), e.g., "America/New_York". This
+      specified by the `IANA Time Zone Database
+      <http://www.iana.org/time-zones>`_, e.g., "America/New_York". This
       attribute is returned by all end points except the Country end point.
 
     """
@@ -215,9 +219,9 @@ class Subdivision(PlaceRecord):
     :ivar geoname_id: This is a GeoName ID for the subdivision. This
       attribute is returned by all end points except Country.
     :ivar iso_code: This is a string up to three characters long
-      contain the subdivision portion of the ISO 3166-2 code
-      (http://en.wikipedia.org/wiki/ISO_3166-2). This attribute is returned
-      by all end points except Country.
+      contain the subdivision portion of the `ISO 3166-2 code
+      <http://en.wikipedia.org/wiki/ISO_3166-2>`_. This attribute is
+      returned by all end points except Country.
     :ivar name: The name of the subdivision based on the languages list
       passed to the constructor. This attribute is returned by all end points.
     :ivar names: A dictionary where the keys are language codes and the
@@ -241,6 +245,7 @@ class Subdivisions(tuple):
     end point.
 
     """
+    #pylint:disable=R0924,W0212,W0142
     def __new__(cls, languages, *subdivisions):
         subdivisions = [Subdivision(languages, **x) for x in subdivisions]
         obj = super(cls, Subdivisions).__new__(cls, subdivisions)
@@ -271,14 +276,14 @@ class Traits(Record):
 
     This class has the following attributes:
 
-    :ivar autonomous_system_number: The autonomous system
-      number (http://en.wikipedia.org/wiki/Autonomous_system_(Internet))
+    :ivar autonomous_system_number: The `autonomous system
+      number <http://en.wikipedia.org/wiki/Autonomous_system_(Internet)>`_
       associated with the IP address. This attribute is only available from
       the City/ISP/Org and Omni end points.
     :ivar autonomous_system_organization: The organization
-      associated with the registered autonomous system number
-      (http://en.wikipedia.org/wiki/Autonomous_system_(Internet)) for the IP
-      address. This attribute is only available from the City/ISP/Org and
+      associated with the registered `autonomous system number
+      <http://en.wikipedia.org/wiki/Autonomous_system_(Internet)>`_ for the
+      IP address. This attribute is only available from the City/ISP/Org and
       Omni end points.
     :ivar domain: The second level domain associated with the
       IP address. This will be something like "example.com" or
@@ -331,7 +336,7 @@ class Traits(Record):
                              'organization',
                              'user_type'])
 
-    def __init__(self, languages=None, **kwargs):
+    def __init__(self, **kwargs):
         for k in ['is_anonymous_proxy', 'is_satellite_provider']:
             kwargs[k] = bool(kwargs.get(k, False))
         super(Traits, self).__init__(**kwargs)

geoip2/webservices.py

@@ -60,14 +60,14 @@
 web service docs.
 
 If the web service returns an explicit error document, this is thrown as a
-GeoIP2WebServiceError exception. If some other sort of error occurs, this is
-thrown as a GeoIP2HTTPError. The difference is that the webservice error
+WebServiceError exception. If some other sort of error occurs, this is
+thrown as a HTTPError. The difference is that the webservice error
 includes an error message and error code delivered by the web service. The
 latter is thrown when some sort of unanticipated error occurs, such as the
 web service returning a 500 or an invalid error document.
 
 If the web service returns any status code besides 200, 4xx, or 5xx, this also
-becomes a GeoIP2HTTPError.
+becomes a HTTPError.
 
 Finally, if the web service returns a 200 but the body is invalid, the client
 throws a GeoIP2Error object.
@@ -98,7 +98,7 @@
 import geoip2.models
 import requests
 from requests.utils import default_user_agent
-from .errors import GeoIP2Error, GeoIP2HTTPError, GeoIP2WebServiceError
+from .errors import GeoIP2Error, HTTPError, WebServiceError
 
 
 class Client(object):
@@ -206,66 +206,66 @@ def _response_for(self, path, model_class, ip_address):
         uri = '/'.join([self._base_uri, path, ip_address])
         response = requests.get(uri, auth=(self.user_id, self.license_key),
                                 headers={'Accept': 'application/json',
-                                         'User-Agent': self._user_agent() })
-        if (response.status_code == 200):
+                                         'User-Agent': self._user_agent()})
+        if (response.status_code == 200):  #pylint:disable=E1103
             body = self._handle_success(response, uri)
             return model_class(body, languages=self.languages)
         else:
             self._handle_error(response, uri)
 
     def _user_agent(self):
         return 'GeoIP2 Python Client v%s (%s)' % (geoip2.__version__,
-                                                 default_user_agent())
+                                                  default_user_agent())
 
     def _handle_success(self, response, uri):
         try:
             return response.json()
-        except ValueError as e:
+        except ValueError as ex:
             raise GeoIP2Error('Received a 200 response for %(uri)s'
                               ' but could not decode the response as '
                               'JSON: ' % locals() +
-                              ', '.join(e.args), 200, uri)
+                              ', '.join(ex.args), 200, uri)
 
     def _handle_error(self, response, uri):
         status = response.status_code
 
         if status >= 400 and status < 499:
             self._handle_4xx_status(response, status, uri)
         elif status >= 500 and status < 599:
-            self._handle_5xx_status(response, status, uri)
+            self._handle_5xx_status(status, uri)
         else:
-            self._handle_non_200_status(response, status, uri)
+            self._handle_non_200_status(status, uri)
 
     def _handle_4xx_status(self, response, status, uri):
         if response.content:
             try:
                 body = response.json()
-            except ValueError as e:
-                raise GeoIP2HTTPError(
+            except ValueError as ex:
+                raise HTTPError(
                     'Received a %(status)i error for %(uri)s but it did'
                     ' not include the expected JSON body: ' % locals() +
-                    ', '.join(e.args), status, uri)
+                    ', '.join(ex.args), status, uri)
             else:
                 if 'code' in body and 'error' in body:
-                    raise GeoIP2WebServiceError(body.get('error'),
-                                                body.get('code'),
-                                                status, uri)
+                    raise WebServiceError(body.get('error'),
+                                          body.get('code'),
+                                          status, uri)
                 else:
-                    raise GeoIP2HTTPError(
+                    raise HTTPError(
                         'Response contains JSON but it does not specify '
                         'code or error keys', status, uri)
         else:
-            raise GeoIP2HTTPError('Received a %(status)i error for %(uri)s '
-                                  'with no body.' % locals(), status, uri)
+            raise HTTPError('Received a %(status)i error for %(uri)s '
+                            'with no body.' % locals(), status, uri)
 
-    def _handle_5xx_status(self, response, status, uri):
-        raise GeoIP2HTTPError('Received a server error (%(status)i) for '
-                              '%(uri)s' % locals(), status, uri)
+    def _handle_5xx_status(self, status, uri):
+        raise HTTPError('Received a server error (%(status)i) for '
+                        '%(uri)s' % locals(), status, uri)
 
-    def _handle_non_200_status(self, response, status, uri):
-        raise GeoIP2HTTPError('Received a very surprising HTTP status '
-                              '(%(status)i) for %(uri)s' % locals(), status,
-                              uri)
+    def _handle_non_200_status(self, status, uri):
+        raise HTTPError('Received a very surprising HTTP status '
+                        '(%(status)i) for %(uri)s' % locals(), status,
+                        uri)
 """
 
 :copyright: (c) 2013 by MaxMind, Inc.

pylintrc

@@ -0,0 +1,6 @@
+[MESSAGES CONTROL]
+disable=R0201,W0105
+
+[BASIC]
+
+no-docstring-rgx=_.*

setup.py

@@ -30,6 +30,7 @@
     package_dir={'geoip2': 'geoip2'},
     include_package_data=True,
     install_requires=['requests'],
+    test_suite="tests",
     license=open('LICENSE').read(),
     classifiers=(
         'Development Status :: 3 - Alpha',