Skip to content
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

fix pip check for optional dependencies #405

Merged
merged 7 commits into from
Feb 14, 2025
Merged

fix pip check for optional dependencies #405

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f6ba2dd
fix pip check for optional dependencies
jan-janssen Feb 14, 2025
be106ca
install optional dependencies
jan-janssen Feb 14, 2025
3893454
Update pipeline.yml
jan-janssen Feb 14, 2025
49bb62e
add new line
jan-janssen Feb 14, 2025
5850d4a
Update check.py
jan-janssen Feb 14, 2025
256ecfc
Update pipeline.yml
jan-janssen Feb 14, 2025
8485643
Update environment-mace.yml
jan-janssen Feb 14, 2025
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

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 and TOML parsing.

The script should handle potential file operation errors and TOML parsing exceptions to prevent crashes and data loss.

Apply this diff to add error handling:

-    with open("pyproject.toml", "r") as f:
-        data = tomlkit.load(f)
+    try:
+        with open("pyproject.toml") 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)

-    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 pyproject.toml: {e}")
+        sys.exit(1)

Don't forget to add the required import:

import sys

Also applies to: 15-16

🧰 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

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for TOML structure and required fields.

The script assumes the presence of required fields in the TOML file without validation. This could lead to KeyError exceptions if the fields are missing.

Apply this diff to add validation:

-    lst = []
-    for sub_lst in data["project"]["optional-dependencies"].values():
-        for el in sub_lst:
-            lst.append(el)
+    lst = []
+    if "project" not in data:
+        print("Error: Missing 'project' section in pyproject.toml")
+        sys.exit(1)
+    if "optional-dependencies" not in data["project"]:
+        print("Error: Missing 'optional-dependencies' in project section")
+        sys.exit(1)
+    if "dependencies" not in data["project"]:
+        print("Error: Missing 'dependencies' in project section")
+        sys.exit(1)
+    
+    for sub_lst in data["project"]["optional-dependencies"].values():
+        for el in sub_lst:
+            lst.append(el)
📝 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
lst = []
for sub_lst in data["project"]["optional-dependencies"].values():
for el in sub_lst:
lst.append(el)
data["project"]["dependencies"] += list(set(lst))
lst = []
if "project" not in data:
print("Error: Missing 'project' section in pyproject.toml")
sys.exit(1)
if "optional-dependencies" not in data["project"]:
print("Error: Missing 'optional-dependencies' in project section")
sys.exit(1)
if "dependencies" not in data["project"]:
print("Error: Missing 'dependencies' in project section")
sys.exit(1)
for sub_lst in data["project"]["optional-dependencies"].values():
for el in sub_lst:
lst.append(el)
data["project"]["dependencies"] += list(set(lst))


with open("pyproject.toml", "w") as f:
f.writelines(tomlkit.dumps(data))
2 changes: 1 addition & 1 deletion .ci_support/environment-qe.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ channels:
- conda-forge
dependencies:
- qe =7.2
- pwtools =1.2.3
- pwtools =1.2.3
13 changes: 10 additions & 3 deletions .github/workflows/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,25 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Conda config
run: echo -e "channels:\n - conda-forge\n" > .condarc
run: |
cp .ci_support/environment.yml environment.yml
tail --lines=+4 .ci_support/environment-lammps.yml >> environment.yml
tail --lines=+4 .ci_support/environment-qe.yml >> environment.yml
tail --lines=+4 .ci_support/environment-gpaw.yml >> environment.yml
echo -e "channels:\n - conda-forge\n" > .condarc
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for environment file existence.

The script assumes the existence of environment files without validation. This could lead to pipeline failures if files are missing.

Apply this diff to add validation:

       run: |
-        cp .ci_support/environment.yml environment.yml
-        tail --lines=+4 .ci_support/environment-lammps.yml >> environment.yml
-        tail --lines=+4 .ci_support/environment-qe.yml >> environment.yml
-        tail --lines=+4 .ci_support/environment-gpaw.yml >> environment.yml
-        echo -e "channels:\n  - conda-forge\n" > .condarc
+        for file in \
+          ".ci_support/environment.yml" \
+          ".ci_support/environment-lammps.yml" \
+          ".ci_support/environment-qe.yml" \
+          ".ci_support/environment-gpaw.yml"; do
+          if [ ! -f "$file" ]; then
+            echo "Error: $file not found"
+            exit 1
+          fi
+        done
+        
+        cp .ci_support/environment.yml environment.yml
+        for file in \
+          ".ci_support/environment-lammps.yml" \
+          ".ci_support/environment-qe.yml" \
+          ".ci_support/environment-gpaw.yml"; do
+          tail --lines=+4 "$file" >> environment.yml || {
+            echo "Error: Failed to append $file"
+            exit 1
+          }
+        done
+        
+        echo -e "channels:\n  - conda-forge\n" > .condarc
📝 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
run: |
cp .ci_support/environment.yml environment.yml
tail --lines=+4 .ci_support/environment-lammps.yml >> environment.yml
tail --lines=+4 .ci_support/environment-qe.yml >> environment.yml
tail --lines=+4 .ci_support/environment-gpaw.yml >> environment.yml
echo -e "channels:\n - conda-forge\n" > .condarc
run: |
for file in \
".ci_support/environment.yml" \
".ci_support/environment-lammps.yml" \
".ci_support/environment-qe.yml" \
".ci_support/environment-gpaw.yml"; do
if [ ! -f "$file" ]; then
echo "Error: $file not found"
exit 1
fi
done
cp .ci_support/environment.yml environment.yml
for file in \
".ci_support/environment-lammps.yml" \
".ci_support/environment-qe.yml" \
".ci_support/environment-gpaw.yml"; do
tail --lines=+4 "$file" >> environment.yml || {
echo "Error: Failed to append $file"
exit 1
}
done
echo -e "channels:\n - conda-forge\n" > .condarc

- name: Setup Mambaforge
uses: conda-incubator/setup-miniconda@v3
with:
python-version: '3.12'
miniforge-version: latest
condarc-file: .condarc
environment-file: .ci_support/environment.yml
environment-file: environment.yml
- name: Pip check
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 +152 to +154
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for script execution.

The script execution should be validated to ensure it completes successfully.

Apply this diff to add error handling:

-        pip install versioneer[toml]==0.29 tomlkit
-        python .ci_support/check.py
-        cat pyproject.toml
+        pip install versioneer[toml]==0.29 tomlkit || {
+          echo "Error: Failed to install dependencies"
+          exit 1
+        }
+        
+        if [ ! -f ".ci_support/check.py" ]; then
+          echo "Error: check.py not found"
+          exit 1
+        fi
+        
+        python .ci_support/check.py || {
+          echo "Error: Failed to process dependencies"
+          exit 1
+        }
+        
+        if [ ! -f "pyproject.toml" ]; then
+          echo "Error: pyproject.toml not found after processing"
+          exit 1
+        fi
+        cat pyproject.toml
📝 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 || {
echo "Error: Failed to install dependencies"
exit 1
}
if [ ! -f ".ci_support/check.py" ]; then
echo "Error: check.py not found"
exit 1
fi
python .ci_support/check.py || {
echo "Error: Failed to process dependencies"
exit 1
}
if [ ! -f "pyproject.toml" ]; then
echo "Error: pyproject.toml not found after processing"
exit 1
fi
cat pyproject.toml

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

Expand Down
Loading