Export VideoCapture from the package root, lazily - #3
Merged
Merged
Conversation
`from videostream2py import VideoCapture` raised ImportError: the package `__init__.py` was a bare docstring with no imports and no `__all__`, so the only working entry point was the fully-qualified `videostream2py.video`. The docstring's own example was marked `# doctest: +SKIP`, and the package had no tests, so nothing caught it. Add `__all__` plus a PEP 562 module-level `__getattr__`/`__dir__` that resolve the name on first access. The re-export is deliberately lazy rather than a plain top-level `from .video import VideoCapture`: importing `videostream2py.video` imports `cv2`, whose Linux wheels need libGL.so.1 at import time (see the [tool.wads.ops.libgl] block in pyproject.toml). An eager re-export would turn a currently-working `import videostream2py` into a hard failure on bare runners and headless containers. Strictly additive: `videostream2py.video.VideoCapture` is untouched and returns the identical object, and the name being added previously raised ImportError for every caller. The import-time dependency footprint of `import videostream2py` is unchanged (still no cv2). Tests: un-skip the docstring example so `--doctest-modules` actually runs it, and add tests/test_exports.py covering root-level import identity, `__all__` /`dir()` membership, the AttributeError path, and a subprocess check that a bare `import videostream2py` leaves cv2 out of sys.modules. Before: `pytest -q` -> no tests ran; `pytest --doctest-modules -q` -> 1 skipped. After: `pytest -q` -> 4 passed; `pytest --doctest-modules -q` -> 5 passed. Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
Review follow-up on the lazy re-export. The subprocess probe that guards laziness did not pin which copy of the package it loaded. Under PYTHONSAFEPATH the cwd is off sys.path for `python -c`, so the child resolved `videostream2py` from whatever installed distribution was around rather than from the tree under test. Reproduced: a checkout carrying the eager `from .video import VideoCapture` reported 5 passed. The probe now prepends the tree under test to PYTHONPATH and reports its own `__file__`, which a test compares against the in-process one, so a subject swap fails loudly instead of passing quietly. The `dir()` assertion was vacuous: by the time it ran, `__getattr__` had already cached the name into `globals()`, so deleting `__dir__` altogether still passed. The public surface is now asserted inside the fresh-import probe, before anything touches an attribute. Also: derive `__all__` from `_LAZY_EXPORTS` instead of repeating it (two hand-maintained copies could drift silently in either direction, and a new export is now genuinely one line); tighten the AttributeError match to the full message rather than just the missing name; assert on returncode instead of `check=True`, whose exception drops the child's stderr; and narrow the docstring claim -- a bare import never touches cv2, but `import *`, `hasattr`, `inspect.getmembers` and `help` do materialise `__all__` and so can fail where the bare import would not. Exclude `videostream2py/tests` from the wheel. The tests sit inside the package dir to satisfy `testpaths`, but they import pytest, a dev-only extra, so shipping them put an unsatisfiable import in the installed distribution. The sdist and the repo still carry them. Verified: each of the three mutants (eager re-export, `__dir__` deleted, degraded AttributeError message) now fails the suite, and the unmutated source passes in a foreign checkout. Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
videostream2py/__init__.pyhad no public API, so the package's only class wasunreachable from the root:
Every sibling in the family (
audiostream2py,pchealthstream2py) re-exports itsreader from the package root, so this was an inconsistency as well as an ergonomics gap.
Closes #1.
Fix
A lazy re-export (PEP 562), driven by one mapping:
plus module-level
__getattr__/__dir__. Resolved names are cached intoglobals()on first access, so the cost is paid once and__getattr__is notconsulted again.
__all__is derived rather than restated: two hand-maintained copies of the sameset could drift in either direction silently — a name in
__all__only would breakimport *for every caller, and a name in the mapping only would be invisible todir(), tab-completion and Sphinx. Adding an export is now one line.Why lazy and not
from .video import VideoCaptureImporting
videostream2py.videoimportscv2, and opencv-python's Linux wheels needlibGL.so.1present at import time — the repo documents this in its own[tool.wads.ops.libgl]block. An eager re-export would turn a currently-workingimport videostream2pyinto a hard failure on bare runners and headless containers.importlib.import_moduleis aliased to_import_moduleso it does not leak into thepackage's public surface.
Back-compat
No name is removed or renamed; nothing that worked before behaves differently.
from videostream2py.video import VideoCaptureis untouched, and the class objectreached through the root is the identical object (
is-checked in the suite).The one honest caveat, now stated in the module docstring rather than glossed over:
a bare
import videostream2pynever imports cv2, but anything that materialises__all__does —import *,hasattr,inspect.getmembers,help(). On a hostwhere
import cv2itself fails, those now raise thatImportErrorwhere previouslythey returned an empty surface. Swallowing it would hide a real failure and an eager
re-export is strictly worse, so the claim is narrowed instead of the code weakened.
This repo's CI is not exposed: both the
validationandgithub-pagesjobs runwads'
install-system-deps, which installs libgl1 from the[tool.wads.ops.libgl]block above.
Tests
New
videostream2py/tests/test_exports.py(in-package, matching the siblingaudiostream2py/audiostream2py/tests/convention thattestpaths = ["videostream2py"]requires), and the docstring example is un-skipped so
--doctest-modulesactuallyexercises it.
The laziness guard runs in a child interpreter pinned to the tree under test via
PYTHONPATH, and the child reports its own__file__for a test to compare againstthe in-process one. Without that pin the guard is decided by the ambient environment
rather than by the code: under
PYTHONSAFEPATHthe cwd is not onsys.pathforpython -c, so the child silently resolves the package from whatever installeddistribution happens to be around. Verified — a checkout carrying the eager
from .video import VideoCapturepassed the unpinned guard.The public-surface assertion is made inside that fresh-import probe rather than
in-process, because in-process it is vacuous:
__getattr__has already cached thename into
globals()by the time any test runs, so plaindir()contains it whetheror not
__dir__exists.Each of these three mutants was confirmed to fail the suite, and the unmutated source
to pass in a foreign checkout:
from .video import VideoCapturetest_importing_the_package_does_not_import_cv2,test_the_public_surface_is_exactly_the_lazy_exports__dir__deletedtest_the_public_surface_is_exactly_the_lazy_exportsAttributeError(name)instead of the full messagetest_unknown_attribute_raises_attribute_error_naming_the_modulePackaging
[tool.hatch.build.targets.wheel] exclude = ["videostream2py/tests"]. The tests liveinside the package dir to satisfy
testpaths, but they import pytest — adev-onlyextra — so without the exclusion the wheel would ship an importable submodule with an
unsatisfiable import, which any
pkgutil.walk_packagesconsumer or doc scanner wouldtrip over. The sdist and the repo keep them.
Baseline
Before:
pytest -q→ "no tests ran";pytest --doctest-modules -q→ 1 skipped.After: 5 passed / 6 passed respectively, green with and without
PYTHONSAFEPATH.The one pre-existing
ruffD100 ondocsrc/conf.pyis untouched and unrelated.https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe