chore: improve python-progressbar maintenance path#314
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the type hints for AttributeDict in progressbar/utils.py to support any value type and introduces a suite of unit tests for color-related utilities and dictionary behavior. The reviewer suggests explicitly importing the utils module in the test file to maintain consistency and recommends expanding the test coverage to include 24-bit TrueColor sequences and byte-based ANSI sequences for length calculations.
| ], | ||
| ) | ||
| def test_no_color(value, expected) -> None: | ||
| assert progressbar.utils.no_color(value) == expected |
There was a problem hiding this comment.
The tests use progressbar.utils, but this submodule is not explicitly imported in the file. Given that progressbar.env is explicitly imported at line 6, it is recommended to also explicitly import progressbar.utils at the top of the file to ensure the tests run correctly and maintain consistency with the existing import style.
| (b'\x1b[31m', b''), | ||
| ('\x1b[1m\x1b[31mtext\x1b[0m', 'text'), | ||
| (b'\x1b[1m\x1b[31mtext\x1b[0m', b'text'), | ||
| ('\x1b[38;5;208mhello world\x1b[0m', 'hello world'), |
| 'value,expected', | ||
| [ | ||
| ('', 0), | ||
| (b'', 0), |
There was a problem hiding this comment.
Summary:
Notes: