Move to modern pyproject.toml for packaging spinup-forecast
#65
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the Python packaging configuration by migrating from the legacy setup.py approach to the modern pyproject.toml standard for the spinup-forecast project. The change also restructures the package from a local lib/ directory to a proper src/ layout with the package name nemo_spinup_forecast.
- Replaces
setup.pyandpytest.iniwith comprehensivepyproject.tomlconfiguration - Restructures package layout from
lib/tosrc/nemo_spinup_forecast/ - Updates all import statements to use the new package structure
- Adds new test functionality for NetCDF file operations
Reviewed Changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Defines modern packaging configuration with dependencies and build system |
| setup.py | Removed legacy setup configuration |
| pytest.ini | Removed in favor of pytest configuration in pyproject.toml |
| tests/test_forecast.py | Updated imports and added NetCDF testing functionality |
| src/nemo_spinup_forecast/restart.py | Removed unused density import |
| main_restart.py | Updated import path from lib/ to nemo_spinup_forecast |
| main.py | Updated import path from lib/ to nemo_spinup_forecast |
Comments suppressed due to low confidence (1)
tests/test_forecast.py:26
- [nitpick] The function name 'create_netcdf4_file' is misleading as it creates a NetCDF file using xarray, not specifically NetCDF4 format. Consider renaming to 'create_netcdf_file' or 'create_test_netcdf'.
def create_netcdf4_file(path: Path):
|
@isaacaka this is now ready for review.
|
isaacaka
left a comment
isaacaka
left a comment
There was a problem hiding this comment.
All tests pass.
In order to run main.py
Need to install in package in edit mode pip install -e . or src to be added to the pythonpath via: PYTHONPATH=src python main.py --ye True --start 20 --end 50 --comp 1 --steps 30 --path /path/to/data
What do you think is the best approach for this?
It seemed to run fine for me and run in the mode you described above with modification of PYTHONPATH. Though you reminded me that it would be great to execute it as a module as we've done with spinup-evaluation. This means we could run it anywhere. |
Reinstalled the environment and works without modification. Good to merge :) |
isaacaka
left a comment
isaacaka
left a comment
There was a problem hiding this comment.
Update README instructions to include additional config file arguments
Python 3.9 has reached end of life so we can bump up our minimum supported version to 3.10
Forecasting pipeline run successfully
All tests pass
- introduce pyproject.toml, update tests and all import statements - fix import paths from lib to src - add NetCDF test to verify file can be loaded and use pythonpath package - use xarray[io] instead of xarray[complete] - bump Pytest version so that pythonpath is compatible - various src cleanup
- chore(lint): lint fixes to forecast.py - chore(lint): update exclude paths in ruff.toml - chore(lint): correct test_forecast.py errors - chore(lint): Fix linting failure
- fix restart.py so that ruff check now passes - converted French notes to English - cleaned up docstrings - annotated unused variables - fixed comments We need to review or update: - the comment referencing Isaac - the update_e3t function - static type hinting - some comments seem out of date / wrong - no testing of any of the functions
- add pre-commit as dev dependency - add lower bounds on main and dev dependencies - remove '-q' so that the user knows how many tests will run and generally outputs the tests results in a nicer format. - bump to Python==3.10 Co-authored-by: isaacaka <[email protected]>
- update docs for module execution - amend README.md
The behaviour now is that the default ocean_terms.DINO.yaml and techniques_config.yaml will always be found, ass they are provided as part of the package. Users can easily override by providing command line arguments: - Enables the path resolution to succeed - Move ocean_terms.yaml and techniques_config.yaml - Introduced CLI flags --ocean-terms and --techniques-config - Simplified technique loaders to accept direct file paths test: update paths to src and point to configs - update paths from lib to src - point to package techniques_config.yaml docs(readme): add cli docs to pipeline description
pyproject.tomlxarray[io]instead ofxarray[complete]. Bump pytest version so that pythonpath is compatible.Closes #64