Repository navigation
Improve the public type hints #470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ed1b79f
cfbf57d
9fe9520
8e852ce
6240558
9fc18b1
45374ed
6d225a6
aeb8f3a
92342e5
0f24bc2
2a8a5f1
c362576
85711f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| from importlib.metadata import version | ||
| from typing import IO, TYPE_CHECKING, cast | ||
| from typing import TYPE_CHECKING, cast | ||
|
|
||
| from .const import ( | ||
| MODE_AUTO, | ||
|
|
@@ -12,12 +12,13 @@ | |
| MODE_MEMORY, | ||
| MODE_MMAP, | ||
| MODE_MMAP_EXT, | ||
| Mode, | ||
| ) | ||
| from .decoder import InvalidDatabaseError | ||
| from .errors import InvalidDatabaseError | ||
| from .reader import Reader | ||
|
|
||
| if TYPE_CHECKING: | ||
| import os | ||
| from .types import DatabaseSource | ||
|
|
||
| try: | ||
| from . import extension as _extension | ||
|
|
@@ -33,13 +34,14 @@ | |
| "MODE_MMAP", | ||
| "MODE_MMAP_EXT", | ||
| "InvalidDatabaseError", | ||
| "Mode", | ||
| "Reader", | ||
| "open_database", | ||
| ] | ||
|
|
||
|
|
||
| def open_database( | ||
| database: str | bytes | int | os.PathLike[str] | os.PathLike[bytes] | IO[bytes], | ||
| database: DatabaseSource, | ||
| mode: int = MODE_AUTO, | ||
| ) -> Reader: | ||
| """Open a MaxMind DB database. | ||
|
|
@@ -84,8 +86,9 @@ def open_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", _extension.Reader(database, mode)) | ||
| # implementation.) The extension accepts only a path. It raises TypeError | ||
| # for a file descriptor or a file object. | ||
| return cast("Reader", _extension.Reader(database, mode)) # type: ignore[arg-type] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This maxminddb.open_database(gzip.open(p))With the extension installed, it raises Overloads on 🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agent reply on behalf of @oschwald. 505bab3 in #471 makes |
||
|
|
||
|
|
||
| __version__ = version("maxminddb") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
| import ipaddress | ||
| from dataclasses import dataclass | ||
| from ipaddress import IPv4Address, IPv4Network, IPv6Address, IPv6Network | ||
| from typing import IO, TYPE_CHECKING, Any | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from maxminddb.const import MODE_AUTO, MODE_FD, MODE_FILE, MODE_MEMORY, MODE_MMAP | ||
| from maxminddb.decoder import Decoder | ||
|
|
@@ -20,11 +20,10 @@ | |
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Iterator | ||
| from os import PathLike | ||
|
|
||
| from typing_extensions import Self | ||
|
|
||
| from maxminddb.types import Record, RecordDict | ||
| from maxminddb.types import DatabaseSource, Record, RecordDict | ||
|
|
||
| _IPV4_MAX_NUM = 2**32 | ||
| _REOPENED = "Attempt to iterate over a reopened MaxMind DB. Create a new iterator." | ||
|
|
@@ -58,7 +57,7 @@ class Reader: | |
|
|
||
| def __init__( | ||
| self, | ||
| database: str | bytes | int | PathLike[str] | PathLike[bytes] | IO[bytes], | ||
| database: DatabaseSource, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docstrings still say that
The PR fixed only the docstring in the extension stub. A user who follows the 🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| mode: int = MODE_AUTO, | ||
| ) -> None: | ||
| """Reader for the MaxMind DB file format. | ||
|
|
@@ -95,7 +94,7 @@ def __init__( | |
|
|
||
| def _load( | ||
| self, | ||
| database: str | bytes | int | PathLike[str] | PathLike[bytes] | IO[bytes], | ||
| database: DatabaseSource, | ||
| mode: int, | ||
| ) -> None: | ||
| # TRY301 is suppressed because the handler only closes the buffer and | ||
|
|
@@ -229,10 +228,12 @@ def get_with_prefix_len( | |
| return self._resolve_data_pointer(pointer), prefix_len | ||
| return None, prefix_len | ||
|
|
||
| def __iter__(self) -> Iterator: | ||
| def __iter__(self) -> Iterator[tuple[IPv4Network | IPv6Network, Record]]: | ||
| return self._iterate(self._generation) | ||
|
|
||
| def _iterate(self, generation: int) -> Iterator: | ||
| def _iterate( | ||
| self, generation: int | ||
| ) -> Iterator[tuple[IPv4Network | IPv6Network, Record]]: | ||
| children = self._generate_children(0, 0, 0) | ||
| while True: | ||
| # Check before the walk resumes and reads more nodes, as the C | ||
|
|
@@ -247,7 +248,12 @@ def _iterate(self, generation: int) -> Iterator: | |
| return | ||
| yield record | ||
|
|
||
| def _generate_children(self, node: int, depth: int, ip_acc: int) -> Iterator: | ||
| def _generate_children( | ||
| self, | ||
| node: int, | ||
| depth: int, | ||
| ip_acc: int, | ||
| ) -> Iterator[tuple[IPv4Network | IPv6Network, Record]]: | ||
| node_count = self._node_count | ||
| bits = self._address_bits | ||
| if node > node_count: | ||
|
|
@@ -344,7 +350,7 @@ def _resolve_data_pointer(self, pointer: int) -> Record: | |
|
|
||
| def _load_buffer( | ||
| self, | ||
| database: str | bytes | int | PathLike[str] | PathLike[bytes] | IO[bytes], | ||
| database: DatabaseSource, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The commit "Remove an unneeded type: ignore" removed only one of the unused ignores. Two more ignores in
🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| mode: int = MODE_AUTO, | ||
| ) -> str: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| filename: Any | ||
|
|
@@ -393,7 +399,7 @@ def close(self) -> None: | |
|
|
||
| self.closed = True | ||
|
|
||
| def __exit__(self, *_) -> None: # noqa: ANN002 | ||
| def __exit__(self, *_: object) -> None: | ||
| self.close() | ||
|
|
||
| def __enter__(self) -> Self: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,18 @@ | ||
| """Types representing database records.""" | ||
| """Types for database records and database arguments.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import TypeAlias | ||
| import os | ||
| from typing import TYPE_CHECKING, AnyStr, Protocol, TypeAlias | ||
|
|
||
| Primitive: TypeAlias = str | bytes | bool | float | int | ||
| if TYPE_CHECKING: | ||
| Primitive: TypeAlias = str | bytes | bytearray | bool | float | int | ||
| """Primitive is a type for a scalar value in a database record.""" | ||
| else: | ||
| # The 3.2.0 definition, so that a subscript such as Record[str] works | ||
| # and means the same at runtime. Keep it when the static alias changes. | ||
| # Type checkers reject the subscript. | ||
| Primitive: TypeAlias = AnyStr | bool | float | int | ||
|
|
||
| RecordList: TypeAlias = list["Record"] | ||
| """RecordList is a type for lists in a database record.""" | ||
|
|
@@ -13,3 +21,18 @@ | |
| """RecordDict is a type for dicts in a database record.""" | ||
|
|
||
| Record: TypeAlias = Primitive | RecordList | RecordDict | ||
| """Record is a type for a value in a database record.""" | ||
|
|
||
| StrOrBytesPath: TypeAlias = str | bytes | os.PathLike[str] | os.PathLike[bytes] | ||
| """StrOrBytesPath is a type for a path to a database file.""" | ||
|
|
||
|
|
||
| class SupportsRead(Protocol): | ||
| """SupportsRead is a type for a binary file object for MODE_FD.""" | ||
|
|
||
| def read(self) -> bytes: | ||
| """Return the remaining bytes.""" | ||
|
|
||
|
|
||
| DatabaseSource: TypeAlias = StrOrBytesPath | int | SupportsRead | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Remove
🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agent reply on behalf of @oschwald. Not changed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
maxminddb.open_database(fd, maxminddb.Mode.FD) # fd is an int
# AttributeError: 'int' object has no attribute 'read' (reader.py:373)The README and the docstrings tell users to pass a file descriptor here. A 🤖 Comment by Claude Opus 5.5.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| """DatabaseSource is a type for the database argument of a reader.""" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # mypy: warn-unused-ignores | ||
| """Static type checks for the public API. | ||
|
|
||
| mypy checks this file in the lint environment. The code does not run. Each | ||
| type: ignore marks a line that must fail the type check. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| import gzip | ||
| import io | ||
| from ipaddress import IPv4Network, IPv6Network | ||
| from pathlib import Path | ||
|
|
||
| from typing_extensions import assert_type | ||
|
|
||
| import maxminddb | ||
| import maxminddb.extension | ||
| from maxminddb.types import Primitive, Record | ||
|
|
||
| reader = maxminddb.open_database("GeoIP2-City.mmdb") | ||
| assert_type(reader.get("1.1.1.1"), Record | None) | ||
| assert_type(reader.get_with_prefix_len("1.1.1.1"), tuple[Record | None, int]) | ||
| for network, record in reader: | ||
| assert_type(network, IPv4Network | IPv6Network) | ||
| assert_type(record, Record) | ||
| assert_type(reader.metadata().search_tree_size, int) | ||
| with maxminddb.open_database(Path("GeoIP2-City.mmdb")) as path_reader: | ||
| assert_type(path_reader, maxminddb.Reader) | ||
|
|
||
| # Record includes the bytearray that the C extension returns for the | ||
| # bytes type. | ||
| value: Record = bytearray(b"\x00") | ||
|
|
||
| # A TypeVar in Primitive would give Record members of type Any. mypy | ||
| # rejects Record[str] even then, so only this check catches it. | ||
| def check_not_generic(primitive: Primitive[str]) -> None: # type: ignore[type-arg] | ||
| pass | ||
|
|
||
| def check_mode_fd(gzip_file: gzip.GzipFile, text_file: io.TextIOWrapper) -> None: | ||
| maxminddb.open_database(gzip_file, maxminddb.Mode.FD) | ||
| maxminddb.open_database(text_file, maxminddb.Mode.FD) # type: ignore[arg-type] | ||
|
|
||
| extension_reader = maxminddb.extension.Reader("GeoIP2-City.mmdb") | ||
| for network, record in extension_reader: | ||
| assert_type(network, IPv4Network | IPv6Network) | ||
| assert_type(record, Record) | ||
| assert_type(extension_reader.metadata().node_byte_size, int) | ||
| maxminddb.extension.Reader(3) # type: ignore[arg-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of
Primitivemembers now exists in three places, and you must keep them the same by hand:types.py:9(theTYPE_CHECKINGbranch)types.py:14(the runtime branch):annotation:If a future change adds a type to only one of them, type checkers, the runtime alias and the docs show three different types, and no test catches it. One definition plus a runtime override removes two of the copies:
🤖 Comment by Claude Opus 5.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agent reply on behalf of @oschwald.
Partly. After 85711f7, the runtime branch is the frozen 3.2.0 definition, not a copy of the static alias, so it must not follow changes to it. The suggested
AnyStr | Primitivewould putbytesback intoRecord[str]at runtime. The docs:annotation:stays the one copy. ENG-5623 removes it together with the split.