Repository navigation
fix(bindings): name the Python root package's public surface in __all__ - #590
Merged
Merged
Conversation
The package ships a PEP 561 marker since #586. Under it a type checker treats a bare `from .x import Y` in `__init__.py` as private, so the marker alone made pyright (and so Pylance) refuse the README's own `from offline_protocol_sdk import ProtocolManager` with reportPrivateImportUsage, and `mypy --strict` report that the module "does not explicitly export" it. Before the marker both accepted it. The root package now lists its public surface in `__all__`: the generated module's own `__all__` plus the eleven hand-written managers, using only the `+=` forms the typing spec requires every checker to read. tests/test_public_surface.py pins the list to the attributes the package exposes, so a manager added without a line in `__all__` fails in CI rather than in a user's editor. The wheel harness gains the refusal case for a package without the marker, which #586 had checked by hand. Measured against the wheel built from this branch: pyright 1.1.407 and mypy 2.4.0 --strict both accept the README import with full types.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #586. A PEP 561 marker changes what counts as exported, not only whether the package is typed.
offline_protocol_sdk/__init__.pyre-exported the eleven hand-written managers asfrom .x import Y # noqa: F401, with no__all__and noas Yalias, and under PEP 561 a type checker treats those as private. So with the marker alone, on the README's own quick start (from offline_protocol_sdk import ProtocolManager):Before the marker both accepted it, since pyright reads an untyped library's source. Pyright is what Pylance runs, so this is the first thing a VS Code user sees after upgrading. The generated names were never affected:
from .offline_protocol import *is a wildcard import and the generated module defines__all__.This PR:
__all__: the generated module's own__all__plus the eleven managers, using only the+=forms the typing spec requires every checker to read;bindings/python/tests/test_public_surface.py, which pins__all__to the attributes the package exposes (non-underscore, non-module), so a manager added to the root without a line in__all__fails in CI rather than in a user's editor;scripts/tests/test-build-wheel.shfor a package without the marker, which fix(bindings): ship the PEP 561 marker so type checkers read the Python package #586 had checked by hand;Related issues
Refs #586.
Type of change
fix: bug fixtest: adding or correcting testsChecklist
CHANGELOG.mdupdated (Unreleased, Fixed; extends the fix(bindings): ship the PEP 561 marker so type checkers read the Python package #586 entry)Breaking changes
None at runtime.
from offline_protocol_sdk import *in an application now brings in exactly the names in__all__; before, it also brought in the submodule names (secure_storage,protocol_manager, ...) that the package's own imports bind. Those stay reachable as attributes.Notes for reviewers
How I verified it:
--stricton a consumer doing the README's imports from the root: both pass with full types, and the only reported error is the consumer's intentional wrong-argument call. The same consumer against the fix(bindings): ship the PEP 561 marker so type checkers read the Python package #586 wheel shows the fourreportPrivateImportUsageandattr-definederrors above.__all__and names it (missing from __all__: ['BleManager']).scripts/tests/test-build-wheel.shon Linux (python:3.12-bookworm, aarch64): all checks pass, including the new "a package without the PEP 561 marker is refused".shellcheck --severity=warningis clean on it.