From c31141de26bc5ffb1249ca749c7d007e6b4d9586 Mon Sep 17 00:00:00 2001 From: wouter bolsterlee Date: Mon, 20 Sep 2021 12:25:22 +0200 Subject: [PATCH 1/3] Expose __all__ to avoid type checking errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mypy reports attr-defined errors (via no_implicit_reexport) because the ‘maxminddb’ module uses implicit re-exports. Adding __all__ makes the exported API surface explicit, which avoids the type checking errors. --- maxminddb/__init__.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/maxminddb/__init__.py b/maxminddb/__init__.py index 582d209..2966eb7 100644 --- a/maxminddb/__init__.py +++ b/maxminddb/__init__.py @@ -20,6 +20,19 @@ from maxminddb.decoder import InvalidDatabaseError from maxminddb.reader import Reader as PyReader +__all__ = [ + "InvalidDatabaseError", + "MODE_AUTO", + "MODE_FD", + "MODE_FILE", + "MODE_MEMORY", + "MODE_MMAP", + "MODE_MMAP_EXT", + "PyReader", # Exposed for type checking b/c return type of open_database() + "Reader", + "open_database", +] + def open_database( database: Union[AnyStr, int, os.PathLike, IO], mode: int = MODE_AUTO From e8a521be6bb063ed5dcf30e77d14a2174227491f Mon Sep 17 00:00:00 2001 From: wouter bolsterlee Date: Mon, 20 Sep 2021 20:53:06 +0200 Subject: [PATCH 2/3] =?UTF-8?q?Improve=20type=20checking:=20return=20?= =?UTF-8?q?=E2=80=98Reader=E2=80=99=20from=20open=5Fdatabase()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This exposes the ‘Reader’ class under its own name (instead of an alias) and uses it as the return type for open_database(). In case the C extension is used, cast that to the same ‘Reader’ type for type checking purposes, since they share an identical API. The benefits are that this avoids an unnecessary Union return type (see e.g. https://github.com/python/mypy/issues/1693) and doesn't require internal types to be exposed with confusing names for type checking purposes only. The ‘Reader’ compat helper is unnecessary and can be dropped: the Python Reader is always available and API compatible. While at it, simplify and isort the imports. --- .pylintrc | 1 + maxminddb/__init__.py | 53 +++++++++++++++++++++++++------------------ 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/.pylintrc b/.pylintrc index 53a1201..dd54a45 100644 --- a/.pylintrc +++ b/.pylintrc @@ -4,3 +4,4 @@ disable=C0330 [BASIC] no-docstring-rgx=_.* +extension-pkg-allow-list=maxminddb.extension diff --git a/maxminddb/__init__.py b/maxminddb/__init__.py index 2966eb7..72cbdef 100644 --- a/maxminddb/__init__.py +++ b/maxminddb/__init__.py @@ -1,8 +1,6 @@ # pylint:disable=C0111 import os -from typing import AnyStr, IO, Union - -import maxminddb.reader +from typing import IO, AnyStr, Union, cast try: import maxminddb.extension @@ -11,14 +9,14 @@ from maxminddb.const import ( MODE_AUTO, - MODE_MMAP, - MODE_MMAP_EXT, + MODE_FD, MODE_FILE, MODE_MEMORY, - MODE_FD, + MODE_MMAP, + MODE_MMAP_EXT, ) from maxminddb.decoder import InvalidDatabaseError -from maxminddb.reader import Reader as PyReader +from maxminddb.reader import Reader __all__ = [ "InvalidDatabaseError", @@ -28,15 +26,15 @@ "MODE_MEMORY", "MODE_MMAP", "MODE_MMAP_EXT", - "PyReader", # Exposed for type checking b/c return type of open_database() "Reader", "open_database", ] def open_database( - database: Union[AnyStr, int, os.PathLike, IO], mode: int = MODE_AUTO -) -> Union[PyReader, "maxminddb.extension.Reader"]: + database: Union[AnyStr, int, os.PathLike, IO], + mode: int = MODE_AUTO, +) -> Reader: """Open a MaxMind DB database Arguments: @@ -52,21 +50,32 @@ def open_database( * MODE_AUTO - tries MODE_MMAP_EXT, MODE_MMAP, MODE_FILE in that order. Default mode. """ + if mode not in ( + MODE_AUTO, + MODE_FD, + MODE_FILE, + MODE_MEMORY, + MODE_MMAP, + MODE_MMAP_EXT, + ): + raise ValueError(f"Unsupported open mode: {mode}") + has_extension = maxminddb.extension and hasattr(maxminddb.extension, "Reader") - if (mode == MODE_AUTO and has_extension) or mode == MODE_MMAP_EXT: - if not has_extension: - raise ValueError( - "MODE_MMAP_EXT requires the maxminddb.extension module to be available" - ) - return maxminddb.extension.Reader(database) - if mode in (MODE_AUTO, MODE_MMAP, MODE_FILE, MODE_MEMORY, MODE_FD): - return PyReader(database, mode) - raise ValueError(f"Unsupported open mode: {mode}") + use_extension = has_extension if mode == MODE_AUTO else mode == MODE_MMAP_EXT + + if not use_extension: + return Reader(database, mode) + if not has_extension: + raise ValueError( + "MODE_MMAP_EXT requires the maxminddb.extension module to be available" + ) -def Reader(database): # pylint: disable=invalid-name - """This exists for backwards compatibility. Use open_database instead""" - return open_database(database) + # The C type exposes the same API as the Python Reader, so for type + # checking purposes, pretend it is one. (Ideally this would be a subclass + # of, or share a common parent class with, the Python Reader + # implementation.) + return cast(Reader, maxminddb.extension.Reader(database, mode)) __title__ = "maxminddb" From 7b28aa64bdbd31040fa0ea2172feb898040138eb Mon Sep 17 00:00:00 2001 From: wouter bolsterlee Date: Wed, 22 Sep 2021 11:17:38 +0200 Subject: [PATCH 3/3] Handle missing extension correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the extension module cannot be imported, the fallback path would trigger ‘NameError: name 'maxminddb' is not defined’. While at it: - Use an underscored name for the optional extension import to hide it a bit from view (e.g. autocomplete in interactive Python shells) - Use relative imports not unnecessarily repeat the module name everywhere --- maxminddb/__init__.py | 22 ++++++++++++---------- tests/reader_test.py | 6 +++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/maxminddb/__init__.py b/maxminddb/__init__.py index 72cbdef..5574cf0 100644 --- a/maxminddb/__init__.py +++ b/maxminddb/__init__.py @@ -2,12 +2,7 @@ import os from typing import IO, AnyStr, Union, cast -try: - import maxminddb.extension -except ImportError: - maxminddb.extension = None # type: ignore - -from maxminddb.const import ( +from .const import ( MODE_AUTO, MODE_FD, MODE_FILE, @@ -15,8 +10,15 @@ MODE_MMAP, MODE_MMAP_EXT, ) -from maxminddb.decoder import InvalidDatabaseError -from maxminddb.reader import Reader +from .decoder import InvalidDatabaseError +from .reader import Reader + +try: + # pylint: disable=import-self + from . import extension as _extension +except ImportError: + _extension = None # type: ignore[assignment] + __all__ = [ "InvalidDatabaseError", @@ -60,7 +62,7 @@ def open_database( ): raise ValueError(f"Unsupported open mode: {mode}") - has_extension = maxminddb.extension and hasattr(maxminddb.extension, "Reader") + has_extension = _extension and hasattr(_extension, "Reader") use_extension = has_extension if mode == MODE_AUTO else mode == MODE_MMAP_EXT if not use_extension: @@ -75,7 +77,7 @@ def open_database( # checking purposes, pretend it is one. (Ideally this would be a subclass # of, or share a common parent class with, the Python Reader # implementation.) - return cast(Reader, maxminddb.extension.Reader(database, mode)) + return cast(Reader, _extension.Reader(database, mode)) __title__ = "maxminddb" diff --git a/tests/reader_test.py b/tests/reader_test.py index 30acdbc..9b9fe5a 100644 --- a/tests/reader_test.py +++ b/tests/reader_test.py @@ -250,8 +250,8 @@ def test_opening_path(self): self.assertEqual(reader.metadata().database_type, "MaxMind DB Decoder Test") def test_no_extension_exception(self): - real_extension = maxminddb.extension - maxminddb.extension = None + real_extension = maxminddb._extension + maxminddb._extension = None with self.assertRaisesRegex( ValueError, "MODE_MMAP_EXT requires the maxminddb.extension module to be available", @@ -259,7 +259,7 @@ def test_no_extension_exception(self): open_database( "tests/data/test-data/MaxMind-DB-test-decoder.mmdb", MODE_MMAP_EXT ) - maxminddb.extension = real_extension + maxminddb._extension = real_extension def test_broken_database(self): reader = open_database(