best-practices: pylint 10.00/10, real docstrings, swap flake8 for pylint in CI#136
Merged
best-practices: pylint 10.00/10, real docstrings, swap flake8 for pylint in CI#136
Conversation
Three small things that were overdue: - Copyright lines now use SPDX-FileCopyrightText: format to match the LICENSES/MIT.txt we added - Two http:// links in the module docstring updated to https:// - Removed a comment explaining # pragma: no cover that had no business living in the module header
pylint.toml existed as a 560-line --generate-toml-config dump. Moved the one non-default setting (py-version = "3.12") into pyproject.toml where it belongs and deleted the rest. Running pylint at 6.93/10 then actually fixing things: parse_string was one 64-statement, 27-branch function handling two completely different parsing modes. Extracted _parse_string_strict and _parse_string_unsafe as private helpers; parse_string is now a thin dispatcher. Should have been that way from the start. listdir's filter= parameter shadowed the built-in filter. Since we're in the 2.0 window anyway, renamed it to glob= (which is what it actually is). filter= still works with a DeprecationWarning pointing at the caller -- same pattern as parse_string_unsafe. Smaller fixes: - struct and warnings moved into unconditional top-level imports - fcntl: inline pylint disable with a note explaining why the platform guards above make it safe at that point - unit_class = None replaces an UnboundLocalError try/except used as flow control (ask me how I know that ends badly) - raise ... from None on StopIteration re-raises -- the internal exception is an implementation detail callers shouldn't see - dropped a try/except ValueError: raise that did nothing at all - removed unnecessary else clauses after returns in two places - cli_script gets a one-line docstring instead of a comment
The parse_string refactor made the split obvious. TestParseStrict covers the default strict=True path plus public entry-point tests (including parse_string_unsafe's deprecation shim). TestParseUnsafe covers strict=False. 314 passing, 2 skipped. Up from 303 before this branch.
- Fix 17x no-else-return across comparison and math operators, system property, unit property, from_other, and best_prefix - Refactor best_prefix: extract nested _resolve_prefix_table helper to bring branch count under threshold; simplify index logic; use f-strings - Add real docstrings to all 32 leaf unit subclasses and 34 to_* methods - Add real docstrings to DISK_GEOMETRY / DISK_GEOMETRY_EX ctypes structs - Convert 2 orphaned bare string statements to # comments - Rename local ord -> result in __or__ to avoid shadowing builtin - Surgical inline pylint: disable comments for intentional API names (bytes parameter, sum/format functions) and platform-guarded imports - pyproject.toml: good-names-rgxs for unit naming conventions (kB, kb, to_KiB, etc.), max-module-lines raised to 2000
pylint at 10.00/10 covers everything pyflakes checked (unused imports, undefined names) and more. pycodestyle is kept — it owns PEP 8 whitespace/indentation checks that pylint doesn't replicate. - requirements.txt: drop flake8, add pylint - Makefile: replace ci-flake8 target with ci-pylint - python.yml: drop flake8 step; add pylint step gated to ubuntu/3.12 (linting is not platform-sensitive, no need to run 15x) - contributing.rst: update Code Style, Components, and make ci sections
grep class matched issubclass() calls too, causing false duplicate detection when test_parse_strict.py was added alongside test_file_size.py (both assert issubclass(..., DeprecationWarning)). Changing to grep "^class" restricts matching to actual class definition lines only.
Test Coverage Report
|
- TestBestPrefixInvalidSystem: cover line 514 (bad system= arg) - TestSystemPropertyInvalidBase: cover line 317 (invalid _base) - TestQueryDeviceCapacityWindowsBody: cover lines 1358-1397 via mock.patch(create=True) injecting ctypes/msvcrt — runs on all platforms - TestQueryCapacityWindowsDriveLetterMock: cover lines 1549-1551 via mocked os.name='nt' and shutil.disk_usage — runs on all platforms - Lines 65-68: add # pragma: no cover to elif os.name == 'nt': import block (import-time, cannot be covered without module reload) Local result: 319 passed, 2 skipped, TOTAL 100% coverage, pylint 10.00/10
Replace freeform 'Copyright © YEAR Tim Case <timbielawa@gmail.com>' with 'SPDX-FileCopyrightText: YEAR[-2026] Tim Case <bitmath@lnx.cx>' across all 26 test files, matching the format already applied to bitmath/__init__.py. Year ranges preserved from original headers.
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
1 participant
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.
Summary
This branch completes a pylint remediation pass on
bitmath/__init__.py, bringing the score from 7.29 to 10.00/10, and updates the CI/toolchain accordingly.Code changes (
bitmath/__init__.py)no-else-returnfixed across all comparison operators (__lt__,__le__,__eq__,__ne__,__gt__,__ge__), math operators (__add__,__sub__,__mul__,__truediv__,__floordiv__,__mod__), and several properties/classmethodsbest_prefixrefactored — extracted nested_resolve_prefix_tablehelper to bring branch count under threshold (was 15/12); simplified index logic; converted%-formatting to f-stringsto_*conversion methodsDISK_GEOMETRY/DISK_GEOMETRY_EXctypes structs#commentsord→resultrename in__or__to avoid shadowing the builtin# pylint: disable=for intentional public API names (bytesparameter,sum/formatfunctions) and platform-guarded conditional imports (msvcrt,stat,ctypes)Toolchain changes
requirements.txt: dropflake8, addpylint— pylint at 10.00/10 covers everything pyflakes checked and moreMakefile: replaceci-flake8target withci-pylintpyproject.toml: add[tool.pylint.basic]withgood-names-rgxsfor intentional naming conventions (unit class names likekB,kb, conversion methods liketo_KiB); raisemax-module-linesto 2000.github/workflows/python.yml: dropflake8step; addpylintstep gated toubuntu-latest/ Python 3.12 (linting is not platform-sensitive)docsite/source/contributing.rst: update Code Style, Components, andmake cisections to reflect the toolchain changeTest status
314 passed, 2 skipped — zero regressions.