Skip to content

fix pip check for optional dependencies #579

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

Merged
merged 1 commit into from
Feb 14, 2025
Merged

fix pip check for optional dependencies #579

Show file tree
Hide file tree
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
16 changes: 16 additions & 0 deletions .ci_support/check.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import tomlkit


if __name__ == "__main__":
with open("pyproject.toml", "r") as f:
data = tomlkit.load(f)
Comment on lines +5 to +6
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for file operations.

The script should handle potential file operation errors and missing TOML keys gracefully.

Apply this diff to add error handling:

-    with open("pyproject.toml", "r") as f:
-        data = tomlkit.load(f)
+    try:
+        with open("pyproject.toml", "r") as f:
+            data = tomlkit.load(f)
+    except FileNotFoundError:
+        print("Error: pyproject.toml not found")
+        sys.exit(1)
+    except tomlkit.exceptions.TOMLKitError as e:
+        print(f"Error parsing pyproject.toml: {e}")
+        sys.exit(1)

Don't forget to add the required import:

import sys
🧰 Tools
🪛 Ruff (0.8.2)

5-5: Unnecessary open mode parameters

Remove open mode parameters

(UP015)


lst = []
for sub_lst in data["project"]["optional-dependencies"].values():
for el in sub_lst:
lst.append(el)

data["project"]["dependencies"] += list(set(lst))
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Validate dependency format before adding.

The script should validate the dependency format before adding it to the main dependencies list.

Run this script to check the dependency format in the codebase:

🏁 Script executed:

#!/bin/bash
# Description: Check dependency format in pyproject.toml
# Expected format: package[extras]>=version or package[extras]==version

rg -A 5 'dependencies\s*=\s*\[' pyproject.toml

Length of output: 137

Action Required: Enforce Dependency Format Validation

Currently, in .ci_support/check.py at line 13, dependencies are appended using:

data["project"]["dependencies"] += list(set(lst))

without any validation. Although the dependency format in pyproject.toml (e.g., "cloudpickle==3.1.1", "pyzmq==26.2.1") adheres to the expected pattern (package[extras]>=version or package[extras]==version), the script must programmatically verify that all dependencies meet this criterion before adding them to the main list.

  • Suggestion: Introduce a validation step—ideally by matching each dependency string against a regular expression—that confirms its format matches the expected pattern. This will prevent potential formatting issues arising from incorrect dependency strings in the future.


with open("pyproject.toml", "w") as f:
f.writelines(tomlkit.dumps(data))
Comment on lines +15 to +16
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for file write operations.

The script should handle potential file write errors gracefully.

Apply this diff to add error handling:

-    with open("pyproject.toml", "w") as f:
-        f.writelines(tomlkit.dumps(data))
+    try:
+        with open("pyproject.toml", "w") as f:
+            f.writelines(tomlkit.dumps(data))
+    except IOError as e:
+        print(f"Error writing to pyproject.toml: {e}")
+        sys.exit(1)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with open("pyproject.toml", "w") as f:
f.writelines(tomlkit.dumps(data))
try:
with open("pyproject.toml", "w") as f:
f.writelines(tomlkit.dumps(data))
except IOError as e:
print(f"Error writing to pyproject.toml: {e}")
sys.exit(1)

4 changes: 3 additions & 1 deletion .github/workflows/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ jobs:
- name: Setup
shell: bash -l {0}
run: |
pip install versioneer[toml]==0.29
pip install versioneer[toml]==0.29 tomlkit
python .ci_support/check.py
cat pyproject.toml
Comment on lines +98 to +100
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Pin tomlkit version and add error handling.

The script execution should be resilient to failures, and dependencies should have fixed versions for reproducibility.

Apply this diff to improve the code:

-        pip install versioneer[toml]==0.29 tomlkit
-        python .ci_support/check.py
-        cat pyproject.toml
+        pip install versioneer[toml]==0.29 tomlkit==0.12.3
+        if ! python .ci_support/check.py; then
+            echo "Error: Failed to process dependencies"
+            exit 1
+        fi
+        if ! cat pyproject.toml; then
+            echo "Error: Failed to display pyproject.toml"
+            exit 1
+        fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pip install versioneer[toml]==0.29 tomlkit
python .ci_support/check.py
cat pyproject.toml
pip install versioneer[toml]==0.29 tomlkit==0.12.3
if ! python .ci_support/check.py; then
echo "Error: Failed to process dependencies"
exit 1
fi
if ! cat pyproject.toml; then
echo "Error: Failed to display pyproject.toml"
exit 1
fi

pip install . --no-deps --no-build-isolation
pip check

Expand Down
Loading