Skip to content

Fix sdist build under setuptools >= 82 (pkg_resources removed) #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sethc555 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix sdist build under setuptools >= 82 (pkg_resources removed) #32

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import setuptools
from pkg_resources import get_distribution
from importlib.metadata import version

try:
get_distribution("setuptools>=39.2.0")
assert tuple(int(x) for x in version("setuptools").split(".")[:2]) >= (39, 2)

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

⚠️ Potential issue | 🟡 Minor

🧩 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 py

Repository: 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.

except Exception as e:
raise AssertionError(
"Please upgrade setuptools by `pip install -U setuptools`: {}".format(
Expand Down