Repo hygiene and full type-check enforcement#241
Conversation
Add mypy and basedpyright tox envs (mirroring the existing pyrefly env) and wire pyrefly,mypy,basedpyright,ty into CI. Fresh envs expose three gaps the dev venv masked: mypy has no ignore-missing-imports override for the optional speedups module, numpy 2.5.1 stubs mistype an update_areas assignment as float64, and newer bundled typeshed makes BufferedWriter generic. Fixed with a mypy override, a cast to the declared alias, and a scoped pyright suppression respectively; stl/_compat.py also restructured to import speedups under private aliases to clear a residual mypy no-redef false positive.
Add a root speedups.pyi stub (matching the real package's signatures)
so basedpyright resolves stl/_compat.py without the optional speedups
extension installed, and force basedpyright's tox env to use its own
interpreter (--pythonpath {envpython}) instead of auto-detecting the
root .venv, which happens to have speedups installed and was masking
the failure CI would hit. Also pin mypy/basedpyright/pyrefly/ty tox
env versions to the currently resolved releases so an untouched repo
can't be redenned by an upstream checker release, and tidy two
CHANGELOG entries (wording + line wrap).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors type annotations and suppresses type checker findings across mypy, basedpyright, pyrefly, and ty, while also replacing the deprecated argparse.FileType in the CLI with safe path-based file opening. Additionally, it removes the legacy build.cmd script and adds a local stub for the optional speedups package. The review feedback correctly points out that the refactored logged decorator in stl/base.py incorrectly uses __name__ (resolving to 'stl.base') instead of class_.__module__, which would misname loggers for classes decorated in other modules. A corresponding update to the test assertions in tests/test_mesh.py is also suggested to align with this fix.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| logger_name: str = '.'.join( | ||
| part.strip() for part in (__name__, class_.__name__) if part.strip() | ||
| ) |
There was a problem hiding this comment.
The logged decorator currently uses __name__ (which resolves to 'stl.base') to construct the logger name. This causes any class decorated with @logged in another module (or in user code) to have its logger incorrectly placed under the stl.base namespace.
To ensure the logger name matches the class's actual defining module (as described in the test comments), use class_.__module__ instead of __name__.
| logger_name: str = '.'.join( | |
| part.strip() for part in (__name__, class_.__name__) if part.strip() | |
| ) | |
| logger_name: str = '.'.join( | |
| part.strip() for part in (class_.__module__, class_.__name__) if part.strip() | |
| ) |
References
- Dynamically determine the logger name using the decorated class's defining module (
class_.__module__) rather than hardcoding the decorator's own module name (__name__) to preserve correct logger hierarchy.
| class _Sample(Logged): | ||
| pass | ||
|
|
||
| assert _Sample.logger.name == 'stl.base._Sample' |
There was a problem hiding this comment.
Update the assertion to verify that the logger name is constructed using the class's actual defining module (_Sample.__module__), aligning with the fix in stl/base.py and the test's own docstring/comment.
| assert _Sample.logger.name == 'stl.base._Sample' | |
| assert _Sample.logger.name == f'{_Sample.__module__}._Sample' |
References
- Ensure test assertions align with the corrected dynamic module-based logger naming scheme.
Summary
Fixes every outstanding type-checker finding at the root and enforces all four declared checkers (pyrefly, mypy, basedpyright, ty) in tox and CI. Also removes legacy packaging leftovers superseded by the 4.0.0 modernization.
Spec:
docs/superpowers/specs/2026-07-06-repo-hygiene-and-type-checks-design.md(local)Type fixes (root-cause, no new blanket suppressions)
stl/__about__.py: single__version__: Final[str]declaration via intermediate — kills mypy no-redef + basedpyright reportRedeclarationstl/base.py:typing.castat 5 numpy stub-imprecision sites (TYP: fix shape-type of structured array fields numpy/numpy#30261); inline logger-name computation replacing name-mangled_Logged__get_nameaccess; explicitdef __hash__consistent with identity__eq__stl/stl.py: scopedpyright: ignorewhere a runtime-defensive isinstance is deliberate (None mesh names) and where newer typeshed genericizedBufferedWriterstl/_compat.py: speedups imports via private aliases assigned intry/else— robust with or without speedups installedstl/ply.py:np.dtype[np.void]parameterization# type: ignorecomments remain instl/CLI: argparse.FileType replaced (deprecated since Python 3.14)
-/omitted still means stdin/stdout binary buffers; unopenable paths still exit 2 viaparser.error_converthelperFileType('wb')truncated the output file at parse time, before the input was validated — in-place conversion (stl f.stl f.stl) destroyed the fileEnforcement
mypy,basedpyright(hermetic via--pythonpath {envpython}),ty; CI type-check job runs all fourspeedups.pyistub so checkers resolve the optional speedups package without it installedspeedupsmirroring the existing pyrefly/ty configHygiene
build.cmd(MSVC 2010 script for the speedups removed in 4.0.0) deleted; ~16 untracked legacy files cleaned from the working tree.python-versiongitignored (was pinning a dead Python 3.7 locally)Verification
Follow-ups (out of scope, noted during review)
stl/base.py:716surfaces as 2 pytest warnings