Quality audit PR 4/6: typing modernization#322
Merged
Conversation
Replace python_utils.types typing aliases with modern spellings: Optional[X] -> X | None, Union -> |, Dict -> dict, Type -> type, Iterable/Iterator/Sequence/MutableSequence/Callable -> collections.abc.*, TypeVar/cast/Any -> typing.*. Drop the now-unused python_utils.types import. Annotation-only; no runtime semantics change.
Modernize typing aliases: Optional[X] -> X | None, Set -> set, Type -> type, and Iterable/Iterator/Sequence/Callable/Generator -> collections.abc.*. Route TYPE_CHECKING/TypeVar through typing; keep python_utils.types only for its own StringTypes alias in utils.py. Annotation-only; no runtime semantics change.
Modernize typing aliases in terminal/base.py and terminal/stream.py: Optional[X] -> X | None, Union -> |, List/Tuple -> list/tuple, Callable/Generator -> collections.abc.*, cast/Any -> typing.*. Drop the now-unused python_utils.types import from base.py. Annotation-only.
Modernize typing aliases: Optional[X] -> X | None, Dict/Tuple -> dict/ tuple, Type -> type, Callable -> collections.abc.Callable, Any -> typing.Any, and route TYPE_CHECKING through typing. Collapse redundant Optional[X | None] color-dict fields to X | None. Drop the now-unused python_utils.types import. Annotation-only; no runtime semantics change.
widgets.py: annotate string_or_lambda/create_marker/create_wrapper, _calculate_eta (ETA/AbsoluteETA), AnimatedMarker.__init__, Bar-family __call__ color: bool, and SamplesMixin.samples (timedelta | int, per its docstring). multi.py: return annotations on __setitem__/__getitem__/stop/ get_sorted_bars/__enter__. utils.py: AttributeDict __getattr__/__setattr__ -> Any, and typed WrappingIO/StreamWrapper listeners as set[ProgressBarMixinBase] (matches start_capturing's parameter type). conftest.py: fixture annotations. Drop the now-obsolete SamplesMixin pyright ignore in tests. Annotation-only.
The shortcut only needs an iterable (ProgressBar.__call__ iterates it), so accept collections.abc.Iterable[T] instead of Iterator[T]. This lets progressbar(range(10)) type-check. Parameter name 'iterator' kept for keyword-caller compatibility. Annotation-only.
The old signature (default: Optional[Type[ValueError]]) was a typing lie (callers pass floats/None as the default). Add @Overloads: with the ValueError sentinel it returns float (or raises); with any other default T it returns float | T. Implementation signature stays permissive (Any), runtime behavior unchanged.
Bump target-version 'py39' -> 'py310'. The only newly-flagged rule is B905 (zip without strict=); add explicit strict=False at the two call sites to preserve the stop-at-shortest behavior (no new exception path). No pyupgrade (UP) autofixes were triggered.
Add tests/test_init_exports.py asserting the three hand-synced lists stay consistent: every _NAME_TO_MODULE name resolves via getattr, __all__'s contents equal the mapping plus the eager dunders, and the TYPE_CHECKING import block (parsed via ast) imports exactly the mapping's names. Reorder __all__ into its canonical form and document _NAME_TO_MODULE as the source of truth. __all__ stays a static literal (kept in RUF022 order) so type checkers still see the re-exports.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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 updates the codebase to target Python 3.10, replacing legacy typing constructs with modern syntax (such as union operators and standard collection types) and adding stricter type annotations across multiple modules. It also introduces tests to ensure initialization exports remain synchronized. The review feedback highlights a critical runtime issue where bytes are written directly to a TextIO stream, and suggests several type-annotation improvements in progressbar/widgets.py to eliminate unnecessary casts and prevent potential runtime TypeErrors.
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.
There was a problem hiding this comment.
Pull request overview
This PR continues the audit series by modernizing type annotations across the project (PEP 604/585/collections.abc), tightening several previously-misleading annotations, and adding a test guard to keep progressbar/__init__.py’s lazy re-export machinery consistent.
Changes:
- Modernize typing throughout core modules and tests (e.g.,
X | None, built-in generics, andcollections.abc.*). - Widen/adjust a few public typings without changing runtime behavior (e.g.,
shortcuts.progressbar()now acceptsIterable[T];deltas_to_secondsgains overloads). - Add a new test to ensure
_NAME_TO_MODULE, theTYPE_CHECKINGimport block, and__all__remain synchronized.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_subclass_compat.py |
Removes now-unnecessary type-check suppression after SamplesMixin.samples typing is corrected. |
tests/test_init_exports.py |
New guard tests for __init__.py export list / lazy mapping consistency. |
tests/conftest.py |
Adds/adjusts fixture annotations and parameter types. |
ruff.toml |
Updates Ruff target-version to py310. |
progressbar/widgets.py |
Broad typing modernization; fixes/expands several widget-related annotations. |
progressbar/utils.py |
Adds overloads for deltas_to_seconds; updates stream-wrapper listener types and other annotations. |
progressbar/terminal/stream.py |
Updates iterator/generator typing imports and annotations. |
progressbar/terminal/base.py |
Typing modernization for terminal/color utilities and related callables/unions. |
progressbar/shortcuts.py |
Widens progressbar() input typing from Iterator[T] to Iterable[T]. |
progressbar/multi.py |
Typing modernization for MultiBar public methods and helpers. |
progressbar/fast.py |
Uses collections.abc.Callable for the native formatter hook typing. |
progressbar/bar.py |
Sweeps core bar types toward PEP 604/585/collections.abc style. |
progressbar/__init__.py |
Adds documentation clarifying __all__ synchronization expectations (and is now test-guarded). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…typing - Replace the TYPE_CHECKING `from .bar import ProgressBarMixinBase` and its listener annotations with a local `_ProgressListener` Protocol, so utils has no dependency on bar (CodeQL flagged the type-checking-only edge as a module-level cyclic import; no runtime cycle existed). Clears both the utils.py and bar.py cyclic-import alerts by removing the only utils->bar edge. - Give the two deltas_to_seconds @overload stubs docstring bodies instead of `...` (CodeQL 'statement has no effect'). - Add `int` to the *deltas parameter type to match the documented/implemented behavior. Annotation-only; runtime unchanged.
The UnicodeEncodeError fallback encoded to bytes and wrote them to the text stream fd (masked by a typing.cast(str, ...)), which raises TypeError at runtime. Decode the ASCII-encoded bytes back to str before writing. Fixes a latent bug in the (uncovered) fallback path.
- create_marker: define _marker inside the isinstance(str) branch, closing over a fresh str local, so the typing.cast(str, marker) is no longer needed (behavior-identical; _marker was only ever wrapped in that branch). - _calculate_eta (ETA + AbsoluteETA): tighten value to float (callers pass a value resolved by _resolve_value_elapsed), dropping the typing.cast(float). - Drop redundant '| None' from terminal.OptionalColor fields (OptionalColor already includes None).
Reference progressbar._NAME_TO_MODULE via an alias instead of mixing `import progressbar` with `from progressbar import ...` (CodeQL py/import-and-import-from), matching tests/test_fast_default.py.
ProgressBarMixinBase.__del__ ran finish() but never chained to a super __del__ (CodeQL 'missing call to superclass __del__', re-surfaced after the ProgressBarBase base-class change). Add a hasattr-guarded super().__del__() call: a no-op today (abc.ABC/object define no __del__) but keeps finalization cooperative and avoids an AttributeError if a base with __del__ is ever added.
…mport-from) Remove the plain 'import collections' that CodeQL flagged as mixing with 'from collections import defaultdict'. Keep the from-import (defaultdict is a public re-export recorded in the API snapshot) and reference defaultdict bare; 'import collections.abc' stays for collections.abc.Callable.
…ing-call-to-delete) CodeQL flagged ProgressBarBase (Iterable + ProgressBarMixinBase multiple inheritance) for finalizing without an explicit __del__ calling the base finalizer. Revert the earlier wrong-class guarded super().__del__() in ProgressBarMixinBase and define ProgressBarBase.__del__ = super().__del__(), which resolves to the same ProgressBarMixinBase.__del__ reached by inheritance — behaviourally identical, MI chain now explicit.
…nherited-attribute) FormatCustomText.__init__ and JobStatusBar.__init__ set instance attributes (format; name/left/right/fill) that their super().__init__ immediately re-sets from the same values, which CodeQL flags as overwriting the inherited attribute. Remove the duplicates; the base initializers (FormatWidgetMixin, Bar, VariableMixin) set them. Verified attrs identical after construction; no behavior change.
…emantics Two attempts to satisfy CodeQL's py/missing-call-to-delete approximation added finalizer complexity with zero behavioral gain: ProgressBarBase defines no __del__ of its own and runtime attribute lookup reaches ProgressBarMixinBase.__del__ unconditionally; CodeQL's static super() model simply cannot follow the MRO past the Iterable hop. Restore the exact develop semantics (teardown-fragile code should stay simple) and dismiss the alert as a false positive instead.
This was referenced Jul 6, 2026
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.
Fourth of six audit PRs (#319, #320, #321). Annotation-only modernization — zero runtime-semantics changes, proven by the API-surface snapshot staying byte-identical (it records parameter names/kinds; only annotations moved).
What changed
types.Optional[X]→X | None,types.Dict→dict,typing.Union→|,types.Callable/Iterable/MutableSequence→collections.abc.*. Thepython_utils.typesalias imports are gone; the codebase now has one way to spell a type._calculate_eta,Bar.__call__,AnimatedMarker, the marker/wrapper factory functions),MultiBar's public methods,AttributeDict(was claimingintvalues), stream-listener sets, conftest fixtures.SamplesMixin.samplesnow admitsint(its own doctest passes one — thetimedelta-only annotation was wrong).progressbar()shortcut widened fromIterator[T]toIterable[T]—progressbar(range(100))and lists now type-check; the parameter name staysiteratorso keyword callers are untouched.deltas_to_secondsgets@overloads replacing thetype[ValueError]-as-sentinel typing lie (runtime behavior identical).target-versionpy39 → py310, matchingrequires-python; newly-flagged pyupgrade rules applied.__init__.pyexport lists guarded: a new test asserts theTYPE_CHECKINGblock, the lazy-import mapping, and__all__stay in sync (they were three hand-maintained lists that could silently drift).Notes
set[ProgressBarMixinBase](notProgressBar) for stream listeners — it matches the actual consumer signature.Verification
574 passed / 100.00% branch coverage (3.14); CI parity (TERM=dumb) green on 3.10 + 3.12; ruff + pyright clean; API snapshot unchanged.