Facelift for reproman to make CI pass and support recent pythons#640
Conversation
- Drop Python 3.9 support, require >= 3.10 - Replace distutils imports with setuptools equivalents (distutils removed in Python 3.12+) - Replace removed setuptools.findall with glob.glob - Switch from distutils.version.LooseVersion to looseversion PyPI package, add it to dependencies - Replace pkg_resources with importlib.metadata for version discovery - Remove broken `python setup.py develop` from tox commands (tox handles package installation) - Add testpaths to pyproject.toml to scope pytest discovery - Add py313, py314 to tox envlist - Pass USER, DBUS_SESSION_BUS_ADDRESS in tox passenv for docker-credential-secretservice compatibility - Handle chardet 6+/7 encoding_era changes with _chardet_detect() wrapper and update test expectations - Fix test_ls_interface to accept both CONNECTION ERROR and NOT FOUND for EC2 resources with fake credentials (varies by boto3 version) Co-Authored-By: Claude Code 2.1.104 / Claude Opus 4.6 <noreply@anthropic.com>
- Drop Python 3.9 from test matrix (now requires >=3.10) - Move datalad/slurm/ssh integration tests to Python 3.10 entry - Remove redundant `python setup.py build` step (deprecated setup_requires/fetch_build_eggs causes CI failures) - Update actions/checkout v2/v3 → v4 - Update actions/setup-python v3/v4 → v5 - Update codecov/codecov-action v1 → v4 Co-Authored-By: Claude Code 2.1.104 / Claude Opus 4.6 <noreply@anthropic.com>
![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 project's minimum Python requirement to 3.10 and migrates from the deprecated distutils library to setuptools and looseversion. It also updates chardet integration to support version 6+ by introducing a wrapper that handles the new encoding_era parameter and improves error handling during unicode decoding. Additionally, test cases were adjusted for EC2 resource reporting and chardet's detection behavior. A review comment suggests hardening the chardet version check in reproman/utils.py to prevent a TypeError when the library is missing or its version cannot be determined.
| if external_versions['chardet'] >= '6': | ||
| from chardet.enums import EncodingEra | ||
| return detect(s, encoding_era=EncodingEra.ALL) | ||
| return detect(s) |
There was a problem hiding this comment.
The comparison external_versions['chardet'] >= '6' will raise a TypeError if chardet is not installed (returning None) or if its version cannot be determined (returning UNKNOWN). Since reproman aims to handle missing external dependencies gracefully, it is safer to check that the version is valid before performing the comparison.
| if external_versions['chardet'] >= '6': | |
| from chardet.enums import EncodingEra | |
| return detect(s, encoding_era=EncodingEra.ALL) | |
| return detect(s) | |
| chardet_ver = external_versions['chardet'] | |
| if chardet_ver and chardet_ver is not external_versions.UNKNOWN and chardet_ver >= '6': | |
| from chardet.enums import EncodingEra | |
| return detect(s, encoding_era=EncodingEra.ALL) | |
| return detect(s) |
There was a problem hiding this comment.
Thanks for the review! This is actually safe here — the from chardet import detect on line 505 guarantees chardet is installed (otherwise ImportError is raised before reaching the version check). Since chardet is a required dependency and always exposes __version__, external_versions['chardet'] reliably returns a LooseVersion, never None or UNKNOWN.
- Run black on changed files to satisfy formatting check - Suppress ref.ref warnings in sphinx conf.py — man page RST files under docs/source/generated/man/ are not committed to git, so references to man page labels are undefined during CI docs build Co-Authored-By: Claude Code 2.1.104 / Claude Opus 4.6 <noreply@anthropic.com>
- Docs: suppress toc.not_readable warnings for ungenerated man pages (previous commit only suppressed ref.ref which is a different warning type) - Lint: reformat all 35 files flagged by latest Black Co-Authored-By: Claude Code 2.1.108 / Claude Opus 4.6 <noreply@anthropic.com>
2 participants
No description provided.