Quality audit PR 5/6: dead code, colors regeneration, major-version backlog#323
Merged
Conversation
The 256-colour table's HSL column was hand-entered and had corrupted rows (e.g. DeepSkyBlue4 #005f87 stored hue 97 where the real hue is 198), so any gradient interpolating through those colours blended through the wrong hue. A handful of lightness values were also off by a rounding step (e.g. NavyBlue 18 -> 19). Add tools/generate_colors.py, which parses the current colors.py to recover the authoritative (binding, RGB, name, xterm) tuples and re-emits every Colors.register(...) line with HSL derived from RGB via HSL.from_rgb. RGB, xterm index, colour name, Python binding name and source order are all preserved verbatim, including the deliberate last-wins duplicate bindings (blue3, deep_sky_blue4, ...); only the HSL column changes. The generator is idempotent (HSL is always recomputed, never read back) and offers --check. tests/test_color.py gains test_registered_color_hls_matches_rgb, which guards every registered colour's stored HSL against HSL.from_rgb(rgb) so the two can never drift apart again.
None of these are part of the public API surface (the snapshot is unaffected): - terminal/base.py: drop the unused ``_CPR`` cursor-position helper (marked ``# pragma: no cover`` / ``reportUnusedClass``) and the now-unused ``threading`` import. ``getch`` was only consumed by ``_CPR`` but is a guarded public re-export, so it is kept via an explicit ``import getch as getch`` re-export. - widgets.py: drop two commented-out pre-migration ``_fixed_colors`` / ``_gradient_colors`` dict declarations left over from the TypedDict migration. - bar.py: drop the commented-out alternative ``custom_len`` annotation (folding its rationale into the docstring) and an obsolete "Python 2.7 / older versions" comment above ``deltas_to_seconds``. - env.py: drop the dead ``if os.name == 'nt': pass # set_console_mode()`` no-op block.
- __init__.py: ``__date__`` was recomputed on every import via ``date.today()`` -- a nondeterministic module attribute and an import-time side effect that nothing consumes. Source it as a static string from __about__ instead (re-exported, still not part of __all__) and drop the now-unused ``from datetime import date`` import. - __about__.py: add ``__date__ = '2024-08-29'`` (release date of 4.5.0, the v4.5.0 tag date; bump on release); lowercase the __url__ host to match pyproject (``github.com/wolph/...``); extend __copyright__ to '2015-2026'. - utils.py: move ``logger = logging.getLogger(__name__)`` from the bottom of the module up next to the TypeVars so it is defined before first use.
Comment/docstring-only clarifications of existing silent-compat behaviour; no code paths and no DeprecationWarnings change (those upgrades are deferred to the next major version, see docs/major-version-backlog.md). - Timer/ETA: explain the legacy bare ``%s`` -> named ``%(elapsed)s`` / ``%(eta)s`` format auto-rewrite. - AdaptiveETA: point readers at `SmoothingETA` for an EMA-based estimate (windowed sample vs exponential moving average). - RotatingMarker / DynamicMessage: document them as legacy aliases kept without a DeprecationWarning until the next major version.
Record the compatibility-breaking changes the quality audit surfaced but deliberately deferred to the next major version, each with a one-paragraph rationale and a pointer to the code: MultiBar dict-subclassing, the FormatLabelBar.__call__ diamond override, the ignored pv-compat CLI flags, the __next__/next iteration path and py2 alias, the ColorBase/WindowsColor no-op public classes, the unused Colors reverse indexes, the 38;5;N SGR form on 16-colour terminals, the deferred DeprecationWarning upgrades, and the deltas_to_seconds ValueError sentinel contract.
|
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}
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
Pull request overview
This PR continues the audit series by removing dead/legacy code, regenerating the terminal 256-color table from a checked-in generator to prevent HSL/RGB drift, and documenting deferred breaking-change candidates for the next major version.
Changes:
- Add
tools/generate_colors.py(+ docs) to regenerateprogressbar/terminal/colors.pywith HSL derived from RGB, and add a regression test lockingcolor.hls == HSL.from_rgb(color.rgb). - Remove dead code and legacy comments/blocks (e.g.,
_CPRcursor-position helper, no-op Windows env block), and adjust a few metadata exports (__date__,__url__). - Add
docs/major-version-backlog.mdcapturing intentionally deferred breaking-change candidates.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/README.md | Documents the developer tooling, including how to run the colors generator. |
| tools/generate_colors.py | New generator that parses colors.py and rewrites register calls with canonical HSL derived from RGB. |
| tests/test_color.py | Adds a regression test asserting registered colors’ HSL is always derived from their RGB. |
| progressbar/widgets.py | Removes commented dead code and adds compatibility-focused documentation notes for legacy shims/aliases. |
| progressbar/utils.py | Moves logger to the import section (no functional change intended). |
| progressbar/terminal/colors.py | Regenerated color table with corrected HSL values (derived from RGB). |
| progressbar/terminal/base.py | Removes _CPR dead code and adjusts exports/imports around it. |
| progressbar/env.py | Removes a no-op Windows branch. |
| progressbar/bar.py | Updates doc/comments related to custom length and legacy notes. |
| progressbar/init.py | Stops recomputing __date__ at import; re-exports static __date__ from __about__. |
| progressbar/about.py | Adds static __date__, updates URL casing, and updates copyright range. |
| docs/major-version-backlog.md | New in-tree backlog for deferred major-version breaking changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Fifth of six audit PRs (#319, #320, #321, #322).
colors.py regenerated from a checked-in generator (bug fix)
The 256-color table carried hand-entered HSL values with systematic corruption (e.g. DeepSkyBlue4 stored hue 97 where the RGB derives ~198) — every gradient interpolating through those colors blended wrong.
tools/generate_colors.pynow emits the module with HSL computed from RGB (the explicit HSL argument is gone entirely —Colors.registerderives it, so the corruption class can't recur), and a new test lockscolor.hls == HSL.from_rgb(color.rgb)for every registered color. Every module-level color name survives, including the historical duplicate last-wins bindings (API snapshot unchanged). The generator is idempotent (second run produces zero diff).Dead code and hygiene
_CPRcursor-position class (+ itsthreadingimport), commented-out alternative declarations, the no-opos.name == 'nt'block in env.py, obsolete py2-era comments.__date__no longer runsdate.today()at import (static, sourced with__about__);__url__casing matches pyproject; copyright range updated;utils.loggermoved up with the imports.%s-format auto-rewrite shims,AdaptiveETA's compat-only smoothing params, and theRotatingMarker/DynamicMessagealiases (no deprecation warnings, per the compat-first policy).docs/major-version-backlog.md (new, committed)
The audit's breaking-change candidates, deliberately NOT changed in this series, now live in-tree with rationales: MultiBar(dict) composition, the FormatLabelBar diamond contract, CLI no-op flags, the ungated
__next__path, no-op color classes, unused color indexes, the 16-color SGR convention, and the deferred DeprecationWarning upgrades.Verification
575 passed / 100.00% branch coverage (3.14); CI parity (TERM=dumb) green on 3.10 + 3.12; ruff + pyright clean; import time unregressed; API snapshot unchanged.