-
Notifications
You must be signed in to change notification settings - Fork 4
update requirement test #647
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.
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
Conversation
WalkthroughThe code was updated so that the list Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.ci_support/check.py (1)
4-16
: Consider adding error handling and documentation.The script lacks error handling for missing keys in the TOML structure and documentation explaining its purpose. Consider adding exception handling for potential KeyError exceptions and a docstring to explain the script's functionality.
import tomlkit if __name__ == "__main__": + """ + Updates project dependencies in pyproject.toml by combining build-system requirements + and optional dependencies, and adding them to the project.dependencies list. + """ with open("pyproject.toml", "r") as f: data = tomlkit.load(f) - lst = list(data['build-system']['requires']) + try: + lst = list(data['build-system']['requires']) + except KeyError: + print("Warning: build-system.requires not found in pyproject.toml") + lst = [] for sub_lst in data["project"]["optional-dependencies"].values(): for el in sub_lst: lst.append(el) - data["project"]["dependencies"] += list(set(lst)) + try: + data["project"]["dependencies"] += list(set(lst)) + except KeyError: + print("Error: project.dependencies not found in pyproject.toml") + exit(1) with open("pyproject.toml", "w") as f: f.writelines(tomlkit.dumps(data))🧰 Tools
🪛 Ruff (0.8.2)
5-5: Unnecessary open mode parameters
Remove open mode parameters
(UP015)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.ci_support/check.py
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: unittest_win
- GitHub Check: unittest_old
- GitHub Check: notebooks
- GitHub Check: notebooks_integration
- GitHub Check: minimal
🔇 Additional comments (1)
.ci_support/check.py (1)
8-8
:✅ Verification successful
Change enhances dependency management by including build requirements.
This change initializes
lst
with build system requirements instead of an empty list, ensuring build dependencies are included in the project dependencies. This makes the dependency management more comprehensive.
🏁 Script executed:
#!/bin/bash # Check if the pyproject.toml file has both build-system.requires and project.dependencies sections if [ -f pyproject.toml ]; then echo "Checking pyproject.toml structure..." # Check if build-system.requires exists and is not empty if ! grep -q "build-system" pyproject.toml || ! grep -q "requires" pyproject.toml; then echo "Warning: build-system.requires section might be missing in pyproject.toml" cat pyproject.toml | grep -A 10 "build-system" fi # Check if project.dependencies exists if ! grep -q "project" pyproject.toml || ! grep -q "dependencies" pyproject.toml; then echo "Warning: project.dependencies section might be missing in pyproject.toml" cat pyproject.toml | grep -A 10 "project" fi else echo "pyproject.toml not found in the current directory" fiLength of output: 253
✅ Confirmed: Build-System Requirements Included
The initialization of
lst
withdata['build-system']['requires']
in.ci_support/check.py
correctly picks up the build dependencies. We’ve verified thatpyproject.toml
contains both a non-empty[build-system].requires
section and a[project].dependencies
section, so this change safely and comprehensively includes build requirements in the dependency checks.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #647 +/- ##
=======================================
Coverage 96.81% 96.81%
=======================================
Files 29 29
Lines 1288 1288
=======================================
Hits 1247 1247
Misses 41 41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit