-
Notifications
You must be signed in to change notification settings - Fork 6
Nlk36701 patch 1 #28
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
base: master
Are you sure you want to change the base?
Nlk36701 patch 1 #28
Conversation
Updated the installation step to include test dependencies.
Not required to run ConcordantModes package, but it is required to run the symmetry tests. The test suite really needs to be updated to reflect this and skip tests when packages aren't detected.
The paths for reference data need to be included.
Updated MANIFEST.in to include additional files and directories for packaging.
Updated checkout action version and adjusted working directory for tests.
There is some change how transf_disp is generating the data. It no longer matches the reference.
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.
Pull request overview
This PR adds a GitHub Actions CI workflow to automate testing across Python 3.9-3.13 and makes necessary packaging modifications in preparation for conda-forge distribution. The changes include setting up build infrastructure (pyproject.toml), updating packaging configurations, and adding a new test file.
- Introduces automated CI testing with GitHub Actions across multiple Python versions
- Updates packaging configuration to properly include test data and dependencies
- Fixes versioneer configuration path formatting in setup.cfg
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/workflow.yml | Adds CI workflow with pytest execution across Python 3.9-3.13 |
| pyproject.toml | Creates build system configuration with setuptools backend |
| setup.py | Adds package_data configuration to include test files and reference data |
| setup.cfg | Removes spaces from versionfile paths for versioneer compatibility |
| MANIFEST.in | Comprehensive rewrite to explicitly include source, tests, and exclude temporary files |
| concordantmodes/tests/_test_transf_disp.py | Adds new test for transformation displacement functionality |
| concordantmodes/tests/init.py | Initializes tests package (empty file) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
.github/workflows/workflow.yml
Outdated
| working-directory: concordantmodes/tests | ||
| run: | | ||
| export PYTHONPATH="${PYTHONPATH}:$(pwd)" | ||
| pytest --cov="ConcordantModes" --color=yes --cov-report=xml |
Copilot
AI
Nov 25, 2025
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.
The coverage argument uses "ConcordantModes" but the package name is "concordantmodes" (lowercase). This mismatch will likely cause coverage tracking to fail. Change --cov="ConcordantModes" to --cov="concordantmodes" to match the actual package name.
| pytest --cov="ConcordantModes" --color=yes --cov-report=xml | |
| pytest --cov="concordantmodes" --color=yes --cov-report=xml |
| "wheel", | ||
| "versioneer" |
Copilot
AI
Nov 25, 2025
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.
versioneer should not be listed in the requires list of [build-system] in pyproject.toml. Versioneer is vendored directly in the repository (versioneer.py file), not installed as a build dependency. Remove "versioneer" from the requires list.
| "wheel", | |
| "versioneer" | |
| "wheel" |
| "concordantmodes": [ | ||
| "tests/*.py", | ||
| "tests/ref_data/*", | ||
| "tests/ref_data/**/*", | ||
| ], |
Copilot
AI
Nov 25, 2025
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.
[nitpick] The package_data dictionary has inconsistent indentation. The keys and values should be properly aligned with standard Python indentation (4 spaces per level). Consider reformatting this section for better readability.
| "concordantmodes": [ | |
| "tests/*.py", | |
| "tests/ref_data/*", | |
| "tests/ref_data/**/*", | |
| ], | |
| "concordantmodes": [ | |
| "tests/*.py", | |
| "tests/ref_data/*", | |
| "tests/ref_data/**/*", | |
| ], |
| recursive-include concordantmodes *.yaml | ||
| recursive-include concordantmodes *.yml | ||
|
|
||
| # junk |
Copilot
AI
Nov 25, 2025
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.
[nitpick] The comment "# junk" is unclear and unprofessional. Consider using a more descriptive comment like "# Exclude temporary and cache files" to better describe what files are being excluded.
| # junk | |
| # Exclude temporary and cache files |
Needless pain and suffering
Added a symlink for ref_data and updated pytest command.
Added a CI GH workflow and made the necessary modifications. One step closer getting this ready for conda-forge.