-
Notifications
You must be signed in to change notification settings - Fork 16
Drop Python 3.9 support and add compatibility for Python 3.13 #721
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
33c4bee
to
5b9627c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #721 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1711 1702 -9
=========================================
- Hits 1711 1702 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 updates the project to drop support for Python 3.9 and add support for newer versions including Python 3.13. It also modernizes type annotations throughout the codebase by replacing legacy typing constructs (e.g. Dict and Tuple) with native PEP 585 generics (e.g. dict and tuple), and updates related tests, documentation, and CI configuration files.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
pyproject.toml, conda-env/*.yml, workflows | Updated Python version requirements and CI matrix to remove 3.9 and support 3.13 |
xcdat/*.py | Converted type annotations to modern native types and removed obsolete future imports |
tests/test_regrid.py | Adjusted test code (e.g. added strict=False in zip) to accommodate new Python behavior |
Comments suppressed due to low confidence (1)
xcdat/regridder/base.py:60
- [nitpick] There is a spelling mistake in the docstring: 'xr.Daatset' should be corrected to 'xr.Dataset'.
Returns
-------
xr.Daatset
- Update `python >=3.10` constraint in conda-env yml files
- No longer needed with python >=3.10
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
f45e84f
to
f031653
Compare
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 pull request upgrades the minimum supported Python version to 3.10 (with added testing for Python 3.13) and modernizes type annotations to the newer PEP 585/604 style across the codebase. The changes span configuration files, workflow definitions, and core modules to ensure compatibility with newer Python versions and cleaner type usage.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
File | Description |
---|---|
xcdat/* | Updated type annotations from typing.Union[...] and related types to the modern union syntax (e.g., “str |
pyproject.toml, conda-env/*, .github/workflows/build_workflow.yml | Updated the supported Python versions and test matrix to reflect the new minimum (3.10) and include Python 3.13. |
Comments suppressed due to low confidence (1)
xcdat/axis.py:395
- np.where returns a tuple of arrays; therefore, using 'len(index_with_360)' always yields the number of dimensions rather than the count of matching elements. Consider updating the check to 'if index_with_360[0].size > 0:' to accurately detect if any indices were found.
if len(index_with_360) > 0:
- Based on GitHub Copilot suggestion
if index_with_360[0].size > 0: | ||
_if_multidim_dask_array_then_load(new_coords) | ||
|
||
new_coords[index_with_360] = 360 |
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.
Updated this logic based on Copilot review comment:
np.where returns a tuple of arrays; therefore, using 'len(index_with_360)' always yields the number of dimensions rather than the count of matching elements. Consider updating the check to 'if index_with_360[0].size > 0:' to accurately detect if any indices were found.
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 upgrades the minimum Python version, modernizes type annotations throughout the codebase, and adds compatibility for Python 3.13. Key changes include:
- Updating CI/CD, Conda environment, and pyproject configuration to require Python >=3.10 and include Python 3.13.
- Converting legacy typing constructs (e.g. Dict, List, Union) to native union and container literals (e.g. dict, list, using the | operator).
- Minor logic improvements such as correcting prime meridian index checks in the longitude axis swap function.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
xcdat/utils.py & xcdat/tutorial.py | Modernized type annotations and removed unnecessary future imports |
xcdat/axis.py | Updated type hints and adjusted the prime meridian check condition |
xcdat/regridder/* | Adopted new union syntax for API type annotations and improved parameter consistency |
conda-env/*, .github/workflows/build_workflow.yml | Updated Python version requirements and added explicit setuptools dependency |
pyproject.toml, docs/conf.py | Modified version constraints and updated type annotations in documentation |
Remaining Files | Consistent modernization of type annotations and minor formatting changes |
Comments suppressed due to low confidence (1)
.github/workflows/build_workflow.yml:58
- The workflow matrix now includes Python 3.13. Please confirm that all dependencies and tests pass under Python 3.13, ensuring full compatibility.
python-version: ["3.10", "3.11", "3.12", "3.13"]
@@ -425,7 +423,7 @@ def _swap_lon_axis(coords: xr.DataArray, to: Tuple[float, float]) -> xr.DataArra | |||
# Example with 360 coords: [60, 150, 0] -> [60, 150, 360] | |||
index_with_360 = np.where(coords == 360) | |||
|
|||
if len(index_with_360) > 0: | |||
if index_with_360[0].size > 0: |
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.
Changing the condition to 'if index_with_360[0].size > 0:' correctly checks for matching indices in a multidimensional np.where result. Please verify that this new condition is intended and consistently applied across similar cases.
Copilot uses AI. Check for mistakes.
@@ -18,10 +19,10 @@ def __init__( | |||
input_grid: xr.Dataset, | |||
output_grid: xr.Dataset, | |||
method: XGCMVerticalMethods = "linear", | |||
target_data: Optional[Union[str, xr.DataArray]] = None, | |||
grid_positions: Optional[Dict[str, str]] = None, | |||
target_data: str | xr.DataArray | None = None, |
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 updated union syntax (using 'str | xr.DataArray | None') improves clarity. Ensure that any downstream modules that depend on this annotation correctly support the new union operator syntax.
Copilot uses AI. Check for mistakes.
FYI @xCDAT/core-developers just merged this PR. The most major code change was replacing all unnecessary |
Adds sparse as a dependency Fixes formatting Updates docstrings [PR]: Enable `skipna` for spatial and temporal mean operations (#655) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> Removes pdb import Fix incorrect dimension used for temporal weights generation (#749) Adds mask creation to create_grid and fixes aligning grid dimension for xesmf Lifting src mask generation Adds create_nan_mask argument to regrid2 Adds docstring to create_mask Add weight threshold option for spatial averaging (#672) - Add parameter `min_weight` to `SpatialAccessor.average()` Replace support section with endorsements (#757) Drop Python 3.9 support and add compatibility for Python 3.13 (#721) Fixes typings Fixes spelling error Fixes black formatting issue Adds scipy dependency Add `.zenodo.json` and `CITATION.cff` to cite core authors (#759) Refactors create_nan_mask Adds tests and fixes mask dimension ordering Fixes variable name Adds create_nan_mask support to xesmf Chunk weights before broadcasting/masking in _group_average (#767) Use the median of the delta instead of min for time freq inference (#768) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> Adds missing test
Adds sparse as a dependency Fixes formatting Updates docstrings [PR]: Enable `skipna` for spatial and temporal mean operations (#655) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> Removes pdb import Fix incorrect dimension used for temporal weights generation (#749) Adds mask creation to create_grid and fixes aligning grid dimension for xesmf Lifting src mask generation Adds create_nan_mask argument to regrid2 Adds docstring to create_mask Add weight threshold option for spatial averaging (#672) - Add parameter `min_weight` to `SpatialAccessor.average()` Replace support section with endorsements (#757) Drop Python 3.9 support and add compatibility for Python 3.13 (#721) Fixes typings Fixes spelling error Fixes black formatting issue Adds scipy dependency Add `.zenodo.json` and `CITATION.cff` to cite core authors (#759) Refactors create_nan_mask Adds tests and fixes mask dimension ordering Fixes variable name Adds create_nan_mask support to xesmf Chunk weights before broadcasting/masking in _group_average (#767) Use the median of the delta instead of min for time freq inference (#768) Co-authored-by: Tom Vo <tomvothecoder@gmail.com> Adds missing test
Description
This pull request focuses on upgrading the minimum supported Python version to 3.10 across the project, adding support for Python 3.13, and modernizing type annotations for improved readability and compliance with newer Python standards.
Python version upgrades:
.github/workflows/build_workflow.yml
to use 3.12 for setup and added 3.13 to the test matrix. [1] [2]conda-env/ci.yml
,conda-env/dev.yml
, andpyproject.toml
. [1] [2] [3]pyproject.toml
.setuptools
explicitly todev.yml
andci.yml
Conda environment files. Required because Python 3.13 requiressetuptools
to be explicitly installed—it is no longer implicitly available in all environments as in previous versions.Type annotation modernization:
Dict
,List
,Optional
,Tuple
, andUnion
with modernPEP 604
andPEP 585
style annotations (e.g.,dict
,list
,|
) across multiple files, includingxcdat/axis.py
,xcdat/bounds.py
, anddocs/conf.py
. [1] [2] [3]Testing improvements:
strict=False
tozip
calls intests/test_regrid.py
to ensure compatibility with the updated Python versions.Logic Improvements:
_swap_lon_axis()
(axis.py)
to check whether coordinates have index with 360 based on GitHub Copilot suggestionChecklist
If applicable: