Skip to content

fix(bindings): name the Python root package's public surface in __all__ - #590

Merged
bahdotsh merged 1 commit into
mainfrom
fix/python-root-exports
Oct 20, 2026
Merged

bahdotsh merged 1 commit into
mainfrom
fix/python-root-exports

Conversation

@bahdotsh

Copy link
Copy Markdown
Member

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__.py re-exported the eleven hand-written managers as from .x import Y # noqa: F401, with no __all__ and no as Y alias, 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):

# pyright 1.1.407, default mode
app.py:2:34 - error: "ProtocolManager" is not exported from module "offline_protocol_sdk"
    Import from "offline_protocol_sdk.protocol_manager" instead (reportPrivateImportUsage)

# mypy 2.4.0 --strict
app.py:2: error: Module "offline_protocol_sdk" does not explicitly export attribute "ProtocolManager"  [attr-defined]

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:

Related issues

Refs #586.

Type of change

  • fix: bug fix
  • test: adding or correcting tests

Checklist

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:

  • Built the wheel from this branch, installed it in a clean venv, and ran pyright 1.1.407 and mypy 2.4.0 --strict on 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 four reportPrivateImportUsage and attr-defined errors above.
  • The whole Python suite against this branch: 1128 passed, 4 skipped. The new guard fails on a copy with one name removed from __all__ and names it (missing from __all__: ['BleManager']).
  • scripts/tests/test-build-wheel.sh on Linux (python:3.12-bookworm, aarch64): all checks pass, including the new "a package without the PEP 561 marker is refused". shellcheck --severity=warning is clean on it.

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.
@bahdotsh
bahdotsh merged commit a8d45ce into main Oct 20, 2026
27 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant