Misc code cleanup and documentation

Author: oschwald

maxminddb/compat.py

@@ -1,5 +1,7 @@
 import sys
 
+# pylint: skip-file
+
 is_py2 = sys.version_info[0] == 2
 
 is_py3_3_or_better = (
@@ -25,12 +27,10 @@ def int_from_bytes(b):
     byte_from_int = chr
 
 else:
-    # XXX -This does apparently slows down the reader by 25 lookups per second
-    # during the search tree lookup. Figure out alternative
     int_from_byte = lambda x: x
 
     FileNotFoundError = FileNotFoundError
 
     int_from_bytes = lambda x: int.from_bytes(x, 'big')
 
-    byte_from_int = lambda x:  bytes([x])
+    byte_from_int = lambda x: bytes([x])

maxminddb/decoder.py

@@ -1,16 +1,30 @@
+"""
+maxminddb.decoder
+~~~~~~~~~~~~~~~~~
+
+This package contains code for decoding the MaxMind DB data section.
+
+"""
 from __future__ import unicode_literals
 
-from struct import pack, unpack
+import struct
 
-from .compat import byte_from_int, int_from_bytes
-from .errors import InvalidDatabaseError
+from maxminddb.compat import byte_from_int, int_from_bytes
+from maxminddb.errors import InvalidDatabaseError
 
 
-class Decoder(object):
+class Decoder(object):  # pylint: disable=too-few-public-methods
 
-    """Decodes the data section of the MaxMind DB"""
+    """Decoder for the data section of the MaxMind DB"""
 
     def __init__(self, database_buffer, pointer_base=0, pointer_test=False):
+        """Created a Decoder for a MaxMind DB
+
+        Arguments:
+        database_buffer -- an mmap'd MaxMind DB file.
+        pointer_base -- the base number to use when decoding a pointer
+        pointer_test -- used for internal unit testing of pointer code
+        """
         self._pointer_test = pointer_test
         self._buffer = database_buffer
         self._pointer_base = pointer_base
@@ -29,15 +43,19 @@ def _decode_bytes(self, size, offset):
         new_offset = offset + size
         return self._buffer[offset:new_offset], new_offset
 
+    # pylint: disable=no-self-argument
+    # |-> I am open to better ways of doing this as long as it doesn't involve
+    #     lots of code duplication.
     def _decode_packed_type(type_code, type_size, pad=False):
+        # pylint: disable=protected-access, missing-docstring
         def unpack_type(self, size, offset):
             if not pad:
                 self._verify_size(size, type_size)
             new_offset = offset + type_size
             packed_bytes = self._buffer[offset:new_offset]
             if pad:
                 packed_bytes = packed_bytes.rjust(type_size, b'\x00')
-            (value,) = unpack(type_code, packed_bytes)
+            (value,) = struct.unpack(type_code, packed_bytes)
             return value, new_offset
         return unpack_type
 
@@ -59,9 +77,9 @@ def _decode_map(self, size, offset):
     def _decode_pointer(self, size, offset):
         pointer_size = ((size >> 3) & 0x3) + 1
         new_offset = offset + pointer_size
-        b = self._buffer[offset:new_offset]
-        packed = b if pointer_size == 4 else pack(
-            b'!c', byte_from_int(size & 0x7)) + b
+        pointer_bytes = self._buffer[offset:new_offset]
+        packed = pointer_bytes if pointer_size == 4 else struct.pack(
+            b'!c', byte_from_int(size & 0x7)) + pointer_bytes
         unpacked = int_from_bytes(packed)
         pointer = unpacked + self._pointer_base + \
             self._pointer_value_offset[pointer_size]
@@ -72,8 +90,8 @@ def _decode_pointer(self, size, offset):
 
     def _decode_uint(self, size, offset):
         new_offset = offset + size
-        b = self._buffer[offset:new_offset]
-        return int_from_bytes(b), new_offset
+        uint_bytes = self._buffer[offset:new_offset]
+        return int_from_bytes(uint_bytes), new_offset
 
     def _decode_utf8_string(self, size, offset):
         new_offset = offset + size
@@ -96,8 +114,13 @@ def _decode_utf8_string(self, size, offset):
     }
 
     def decode(self, offset):
+        """Decode a section of the data section starting at offset
+
+        Arguments:
+        offset -- the location of the data structure to decode
+        """
         new_offset = offset + 1
-        (ctrl_byte,) = unpack(b'!B', self._buffer[offset:new_offset])
+        (ctrl_byte,) = struct.unpack(b'!B', self._buffer[offset:new_offset])
         type_num = ctrl_byte >> 5
         # Extended type
         if not type_num:
@@ -108,7 +131,7 @@ def decode(self, offset):
         return self._type_dispatch[type_num](self, size, new_offset)
 
     def _read_extended(self, offset):
-        (next_byte,) = unpack(b'!B', self._buffer[offset:offset + 1])
+        (next_byte,) = struct.unpack(b'!B', self._buffer[offset:offset + 1])
         type_num = next_byte + 7
         if type_num < 7:
             raise InvalidDatabaseError(
@@ -134,10 +157,11 @@ def _size_from_ctrl_byte(self, ctrl_byte, offset, type_num):
         # Using unpack rather than int_from_bytes as it is about 200 lookups
         # per second faster here.
         if size == 29:
-            size = 29 + unpack(b'!B', size_bytes)[0]
+            size = 29 + struct.unpack(b'!B', size_bytes)[0]
         elif size == 30:
-            size = 285 + unpack(b'!H', size_bytes)[0]
+            size = 285 + struct.unpack(b'!H', size_bytes)[0]
         elif size > 30:
-            size = unpack(b'!I', size_bytes.rjust(4, b'\x00'))[0] + 65821
+            size = struct.unpack(
+                b'!I', size_bytes.rjust(4, b'\x00'))[0] + 65821
 
         return size, offset + bytes_to_read

maxminddb/errors.py

@@ -1,3 +1,6 @@
+"""This module contains custom errors for the MaxMind DB reader"""
+
+
 class InvalidDatabaseError(RuntimeError):
 
     """This error is thrown when unexpected data is found in the database."""

maxminddb/reader.py

@@ -1,11 +1,18 @@
+"""
+maxminddb.reader
+~~~~~~~~~~~~~~~~
+
+This module contains the pure Python database reader and related classes.
+
+"""
 from __future__ import unicode_literals
 
 import mmap
-from struct import unpack
+import struct
 
-from .compat import byte_from_int, int_from_byte, ipaddress
-from .decoder import Decoder
-from .errors import InvalidDatabaseError
+from maxminddb.compat import byte_from_int, int_from_byte, ipaddress
+from maxminddb.decoder import Decoder
+from maxminddb.errors import InvalidDatabaseError
 
 
 class Reader(object):
@@ -23,8 +30,9 @@ class Reader(object):
     def __init__(self, database):
         """Reader for the MaxMind DB file format
 
-        The file passed to it must be a valid MaxMind DB file such as a GeoIP2
-        database file.
+        Arguments:
+        database -- A path to a valid MaxMind DB file such as a GeoIP2
+                    database file.
         """
         with open(database, 'r+b') as db_file:
             self._buffer = mmap.mmap(
@@ -41,25 +49,31 @@ def __init__(self, database):
         metadata_start += len(self._METADATA_START_MARKER)
         metadata_decoder = Decoder(self._buffer, metadata_start)
         (metadata, _) = metadata_decoder.decode(metadata_start)
-        self._metadata = Metadata(**metadata)
+        self._metadata = Metadata(**metadata)  # pylint: disable=star-args
 
         self._decoder = Decoder(self._buffer, self._metadata.search_tree_size
                                 + self._DATA_SECTION_SEPARATOR_SIZE)
 
     # XXX - consider making a property
     def metadata(self):
+        """Return the metadata associated with the MaxMind DB file"""
         return self._metadata
 
     # XXX - change to lookup?
     def get(self, ip_address):
-        """Look up ip_address in the MaxMind DB"""
-        ip = ipaddress.ip_address(ip_address)
+        """Return the record for the ip_address in the MaxMind DB
+
 
-        if ip.version == 6 and self._metadata.ip_version == 4:
+        Arguments:
+        ip_address -- an IP address in the standard string notation
+        """
+        address = ipaddress.ip_address(ip_address)
+
+        if address.version == 6 and self._metadata.ip_version == 4:
             raise ValueError('Error looking up {0}. You attempted to look up '
                              'an IPv6 address in an IPv4-only database.'.format(
                                  ip_address))
-        pointer = self._find_address_in_tree(ip)
+        pointer = self._find_address_in_tree(address)
 
         return self._resolve_data_pointer(pointer) if pointer else None
 
@@ -93,7 +107,7 @@ def _start_node(self, length):
             return self._ipv4_start
 
         node = 0
-        for i in range(96):
+        for _ in range(96):
             if node >= self._metadata.node_count:
                 break
             node = self._read_node(node, 0)
@@ -108,7 +122,7 @@ def _read_node(self, node_number, index):
             offset = base_offset + index * 3
             node_bytes = b'\x00' + self._buffer[offset:offset + 3]
         elif record_size == 28:
-            (middle,) = unpack(
+            (middle,) = struct.unpack(
                 b'!B', self._buffer[base_offset + 3:base_offset + 4])
             if index:
                 middle &= 0x0F
@@ -123,7 +137,7 @@ def _read_node(self, node_number, index):
         else:
             raise InvalidDatabaseError(
                 'Unknown record size: {0}'.format(record_size))
-        return unpack(b'!I', node_bytes)[0]
+        return struct.unpack(b'!I', node_bytes)[0]
 
     def _resolve_data_pointer(self, pointer):
         resolved = pointer - self._metadata.node_count + \
@@ -137,18 +151,37 @@ def _resolve_data_pointer(self, pointer):
         return data
 
     def close(self):
+        """Closes the MaxMind DB file and returns the resources to the system"""
         self._buffer.close()
 
 
 class Metadata(object):
 
+    """Metadata for the MaxMind DB reader"""
+
+    # pylint: disable=too-many-instance-attributes
     def __init__(self, **kwargs):
-        self.__dict__.update(kwargs)
+        """Creates new Metadata object. kwargs are key/value pairs from spec"""
+        # Although I could just update __dict__, that is less obvious and it
+        # doesn't work well with static analysis tools and some IDEs
+        self.node_count = kwargs['node_count']
+        self.record_size = kwargs['record_size']
+        self.ip_version = kwargs['ip_version']
+        self.database_type = kwargs['database_type']
+        self.languages = kwargs['languages']
+        self.binary_format_major_version = kwargs[
+            'binary_format_major_version']
+        self.binary_format_minor_version = kwargs[
+            'binary_format_minor_version']
+        self.build_epoch = kwargs['build_epoch']
+        self.description = kwargs['description']
 
     @property
     def node_byte_size(self):
+        """The size of a node in bytes"""
         return self.record_size // 4
 
     @property
     def search_tree_size(self):
+        """The size of the search tree"""
         return self.node_count * self.node_byte_size