Fix sdist build under setuptools >= 82 (pkg_resources removed)#32
Fix sdist build under setuptools >= 82 (pkg_resources removed)#32sethc555 wants to merge 1 commit into
Conversation
…>=82) setuptools 82.0.0 (2026-02-08) removed pkg_resources, so building this project's sdist in a default isolated build environment fails with ModuleNotFoundError: No module named 'pkg_resources'. Use importlib.metadata (stdlib since 3.8) instead of pkg_resources for the setuptools version check. Verified: the patched sdist builds a wheel with setuptools 82 in a clean python:3.12 container; the unpatched one does not. Co-Authored-By: Claude Fable 5 <[email protected]>
WalkthroughThe ChangesSetuptools version validation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@setup.py`:
- Line 5: The version parsing on the assert line using tuple(int(x) for x in
version("setuptools").split(".")[:2]) can raise ValueError for non-numeric
components; replace this manual parse with a robust PEP 440-aware check (e.g.,
use packaging.version.parse or pkg_resources.parse_version to obtain a Version
object and compare its base numeric components or major/minor attributes) or
wrap the int() conversion in a try/except that treats parse failures as "too
old" and triggers the same upgrade message; update the assertion logic that
references version("setuptools") so it no longer directly casts split parts to
int.
- Line 5: The setuptools minimum-version check in setup.py currently asserts
tuple(int(x) for x in version("setuptools").split(".")[:2]) >= (39, 2) which
conflicts with pyproject.toml's setuptools>=40.8.0; update the assertion to
require (40, 8) (i.e., change the RHS tuple in that assertion) so it matches
pyproject.toml, or if there is an intentional fallback, add a clear comment
above the assert explaining why a lower 39.2 check is kept and reference the
pyproject.toml requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| try: | ||
| get_distribution("setuptools>=39.2.0") | ||
| assert tuple(int(x) for x in version("setuptools").split(".")[:2]) >= (39, 2) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Version parsing may fail on non-standard version strings.
The manual parsing logic split(".")[:2] followed by int(x) will raise ValueError if setuptools has a non-standard version with non-numeric components in the first or second position (e.g., "82a1.0").
While setuptools follows PEP 440 versioning (where pre-release identifiers appear after numeric components), the broad except Exception handler on line 6 will catch any parsing failures and re-raise with the upgrade message, which is reasonable fallback behavior.
🔧 Optional: More robust version parsing with packaging library
If packaging is already a project dependency, consider using it for PEP 440-compliant version parsing:
-from importlib.metadata import version
+from importlib.metadata import version as get_version
+from packaging.version import Version
try:
- assert tuple(int(x) for x in version("setuptools").split(".")[:2]) >= (39, 2)
+ assert Version(get_version("setuptools")) >= Version("39.2.0")This would handle all PEP 440 version formats correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@setup.py` at line 5, The version parsing on the assert line using
tuple(int(x) for x in version("setuptools").split(".")[:2]) can raise ValueError
for non-numeric components; replace this manual parse with a robust PEP
440-aware check (e.g., use packaging.version.parse or
pkg_resources.parse_version to obtain a Version object and compare its base
numeric components or major/minor attributes) or wrap the int() conversion in a
try/except that treats parse failures as "too old" and triggers the same upgrade
message; update the assertion logic that references version("setuptools") so it
no longer directly casts split parts to int.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for setuptools version requirements across the codebase
echo "=== setup.py version check ==="
rg -n "39.*2" setup.py
echo -e "\n=== pyproject.toml build requirements ==="
rg -n "setuptools" pyproject.toml
echo -e "\n=== Any other version requirements ==="
rg -n "setuptools.*>=.*[0-9]" --type pyRepository: youknowone/wirerope
Length of output: 452
Align setuptools minimum version enforcement between setup.py and pyproject.toml.
setup.py asserts setuptools >= (39, 2) (setup.py:5) while pyproject.toml requires setuptools>=40.8.0 (pyproject.toml:2). Update the assertion to 40.8.0 or document why the fallback check intentionally allows a lower version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@setup.py` at line 5, The setuptools minimum-version check in setup.py
currently asserts tuple(int(x) for x in version("setuptools").split(".")[:2]) >=
(39, 2) which conflicts with pyproject.toml's setuptools>=40.8.0; update the
assertion to require (40, 8) (i.e., change the RHS tuple in that assertion) so
it matches pyproject.toml, or if there is an intentional fallback, add a clear
comment above the assert explaining why a lower 39.2 check is kept and reference
the pyproject.toml requirement.
setuptools 82.0.0 (2026-02-08) removed
pkg_resources. Because pip's default isolated build installs the latest setuptools, installing wirerope from sdist now fails:This swaps the pkg_resources setuptools-version check for stdlib importlib.metadata (available since Python 3.8), preserving the original error message.
Verified: with this patch the sdist builds a wheel against setuptools 82 in a clean python:3.12 container; without it, it does not.
Found while scanning the top-5000 PyPI packages for breakage from the setuptools 82 removals — happy to adjust the patch however you prefer.
Summary by CodeRabbit