Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
===========================================
- Coverage 100.00% 97.05% -2.95%
===========================================
Files 8 27 +19
Lines 313 1629 +1316
Branches 0 60 +60
===========================================
+ Hits 313 1581 +1268
- Misses 0 29 +29
- Partials 0 19 +19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR restructures the Python portion of the repo to be packaged as green-grids, updates imports from the prior sparse_grid layout, and adds CI/coverage scaffolding in preparation for PyPI publishing.
Changes:
- Renamed Python package/module references from
sparse_gridtogreen_gridsacross library code, tests, scripts, and examples. - Added initial Python packaging metadata (
pyproject.toml) and CI workflow for linting, testing, and coverage upload. - Added/updated basic utilities and basis abstractions under
python/green_grids/.
Reviewed changes
Copilot reviewed 21 out of 25 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
python/tests/test_utils.py |
Updates test imports to green_grids. |
python/tests/test_transform.py |
Updates test imports to green_grids. |
python/tests/test_sparse_data.py |
Updates test imports to green_grids. |
python/tests/test_repn_transformer.py |
Updates test imports to green_grids. |
python/tests/test_module_functions.py |
Updates test imports to green_grids. |
python/tests/test_generator.py |
Updates test imports to green_grids. |
python/tests/test_basis.py |
Updates test imports to green_grids. |
python/tests/OUTPUT |
Adds a captured pytest run output (currently includes failures). |
python/green_grids/utils.py |
Adds HDF5 dict save/load utilities and checkreal. |
python/green_grids/transform.py |
Updates imports to green_grids. |
python/green_grids/sparse_data.py |
Updates imports to green_grids. |
python/green_grids/repn/transformer.py |
Updates imports to green_grids. |
python/green_grids/repn/ir.py |
Updates imports and tweaks IR basis setup (default grid assignment formatting). |
python/green_grids/repn/generator.py |
Updates imports to green_grids. |
python/green_grids/repn/chebyshev.py |
Updates imports to green_grids. |
python/green_grids/repn/basis.py |
Introduces a new SparseBasis abstraction and sampling/transform helpers. |
python/green_grids/__init__.py |
Updates top-level imports to green_grids and provides public API helpers. |
python/generate.py |
Updates script imports to green_grids. |
python/examples/generate_and_transform.ipynb |
Updates example imports and notebook metadata. |
pyproject.toml |
Adds packaging metadata and scikit-build/cibuildwheel/coverage config. |
README.md |
Adds installation guidance and updates dependency list. |
.gitignore |
Adds Python build/test artifacts to ignore list. |
.github/workflows/python_app.yaml |
Adds CI workflow for linting, testing, and coverage upload. |
.github/codecov.yml |
Updates ignore rules to exclude Python tests from coverage. |
Comments suppressed due to low confidence (3)
python/green_grids/init.py:2
- While updating imports to
green_grids, there are still public docstrings below that reference the oldsparse_gridmodule name (and have atranformertypo). Since this file defines the public API, please update those references so users readinghelp(green_grids.*)get accurate guidance.
python/green_grids/repn/generator.py:3 - This module still emits the warning string "Bose (Fermi) must have even (odd) points" later in the file, which is misleading (Fermi should be even, Bose should be odd). Since this file was touched in the rename, please fix the warning text here too (and ideally avoid duplicating this validation logic by delegating to
SparseBasis.check_ncoeff).
python/green_grids/repn/ir.py:16 - This mutates
sparse_irglobal state (PiecewiseLegendreFT._DEFAULT_GRID) inside theBasisconstructor, which can lead to hard-to-debug cross-test interference and non-local behavior (every instantiation resets the global). Prefer moving this to module import time (guarded so it runs once) or making it configurable without touching a private global.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 46 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 46 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
python/green_grids/init.py:6
__all__only exports__version__, even though this module defines the main public API (get_generator,get_transformer, etc.). This makesfrom green_grids import *unexpectedly drop the functional API; consider either removing__all__or including the intended public symbols.
python/green_grids/sparse_data.py:287- Only
PairedSparseData.save_hdf5()writes the__grids_version__file attribute;SparseData.save_hdf5()does not. If version-gating is enforced on the C++ side, single-grid files produced by Python may be unversioned and fail (or, with the current C++ fallback, be treated as compatible). Consider writing__grids_version__forSparseData.save_hdf5()as well to keep file formats consistent.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… ignore not-slow in buildwheel
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 47 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… library without c++ extensions
iskakoff
left a comment
There was a problem hiding this comment.
Have couple minor questions, otherwise looks good
| " is outdated. Minimum required version is " + GRIDS_MIN_VERSION + "."); | ||
| } | ||
| } else { | ||
| set_version(GRIDS_MIN_VERSION); |
There was a problem hiding this comment.
Not sure about that. If you read file that does not have version why the current version should be set as GRIDS_MIN_VERSION when it is clearly predates GRIDS_MIN_VERSION?
There was a problem hiding this comment.
Falling back to 0.2.4 simply to keep a value that is consistent with the previous release tag: https://github.com/Green-Phys/green-grids/releases/tag/v0.2.4
Do you have any alternative suggestion? If we set the grids version to an empty string or, say, "0.0.0", then that will trigger error with green::grids::CheckVersion
|
Merging for now; alternative ideas for version fallback can be implemented later. |
Introduced some restructuring for the python repo, added GitHub actions workflow for continuous testing and coverage report for python package. Next step will be to deploy the repo as a PyPI package once #7 is fixed.