-
Notifications
You must be signed in to change notification settings - Fork 25
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
Drop setup.py #363
Drop setup.py #363
Conversation
WalkthroughThe pull request introduces significant changes to the project's packaging and configuration. The Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 1
🧹 Nitpick comments (1)
pyproject.toml (1)
134-136
: Consider updating the pytest minimum version requirement.The current minimum version (6.0) is quite old. Pytest 7.x offers improved features and bug fixes. Consider updating to a more recent minimum version.
-minversion = 6.0 +minversion = 7.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pyproject.toml
(1 hunks)setup.py
(0 hunks)
💤 Files with no reviewable changes (1)
- setup.py
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: test (3.12, windows-latest)
- GitHub Check: test (3.12, macos-latest)
- GitHub Check: test (3.12, ubuntu-latest)
- GitHub Check: test (3.11, windows-latest)
- GitHub Check: test (3.11, macos-latest)
- GitHub Check: test (3.11, ubuntu-latest)
- GitHub Check: test (3.10, windows-latest)
- GitHub Check: test (3.10, macos-latest)
- GitHub Check: test (3.10, ubuntu-latest)
🔇 Additional comments (1)
pyproject.toml (1)
124-132
: Setuptools configuration looks good, but let's verify data files inclusion.The configuration correctly sets up package discovery and data files inclusion. The patterns for package data look comprehensive for a scientific package.
Let's verify that all necessary data files are included in the patterns:
✅ Verification successful
All data files are properly covered by the package data patterns
The current configuration in pyproject.toml correctly includes all existing data files in the smact/data directory, including those in the species_rep subdirectory. No files are being missed by the specified patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any data files that might be missed by the current patterns # Find all files in the smact/data directory echo "Files in smact/data directory:" fd . "smact/data" -t f # Check if there are any files that don't match the configured patterns echo -e "\nFiles that might be missed by current patterns:" fd . "smact/data" -t f -E "*.txt" -E "*.csv" -E "*.data" -E "*.xlsx" -E "*.json"Length of output: 1363
[tool.setuptools] | ||
include-package-data = true | ||
|
||
[tool.setuptools.packages.find] | ||
where = ["."] | ||
include = ["smact","smact.*"] | ||
|
||
[tool.setuptools.package-data] | ||
"smact" = ["data/*.txt", "data/*.csv", "data/*.data", "data/*.xlsx", "data/*.json", "data/species_rep/*.json"] | ||
|
||
[tool.pytest.ini_options] | ||
minversion = 6.0 | ||
testpaths = ["smact/tests"] |
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.
💡 Codebase verification
Migration from setup.py is incomplete
Essential configurations are missing in pyproject.toml:
- Package metadata (name, version, description, author details, etc.)
- Dependencies list (scipy, numpy, spglib, etc.)
- Python version requirements (>=3.10)
- Project classifiers
Add these under the [project]
table in pyproject.toml. Example:
[project]
name = "SMACT"
version = "3.0.2"
description = "Semiconducting Materials by Analogy and Chemical Theory"
requires-python = ">=3.10"
dependencies = [
"scipy",
"numpy<3",
# ... other dependencies
]
🔗 Analysis chain
Verify completeness of setup.py migration.
The migration to pyproject.toml appears complete, but let's verify that no configurations were missed from the old setup.py.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any setup.py configurations that might need migration
# Look for the old setup.py in git history
echo "Checking setup.py configurations in last commit:"
git show HEAD^:setup.py 2>/dev/null || echo "setup.py not found in previous commit"
# Check for any remaining setup.py files
echo -e "\nChecking for any remaining setup.py files:"
fd "setup.py"
Length of output: 2733
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #363 +/- ##
========================================
Coverage 75.47% 75.47%
========================================
Files 31 31
Lines 2642 2642
========================================
Hits 1994 1994
Misses 648 648 ☔ View full report in Codecov by Sentry. |
Drop setup.py
Description
This PR completes the migration from setup.py to pyproject.toml
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Test Configuration:
Reviewers
N/A
Checklist
Summary by CodeRabbit
Build Configuration
pyproject.toml
Project Structure
setup.py
filepyproject.toml
Testing