Skip to content

Conversation

sol1105
Copy link
Contributor

@sol1105 sol1105 commented Aug 13, 2025

Pull Request Checklist:

What kind of change does this PR introduce?:

  • Fixed regrid.ipynb
  • Removed need for psymaps / psyplot dependency
  • utils/common.py:
    • Updated require_module to accept an interval of unsupported versions
  • utils/dataset_utils.py:
    • Added option to get_coord_by_type to suppress warning about a main variable not being identifiable
  • utils/output_utils.py:
    • Added functions fix_netcdf_attrs_encoding and '_fix_str_encoding to fix utf-8 encoding issues for global and variable attributes
  • ops/base_operation.py:
    • Applying above fix to correct utf-8 encoding issues
  • core/regrid.py:
    • xarray version warning correctly triggered for versions outside of compatible version interval
    • Grid.detect_extent:
      • determins now also extent in latitude
      • using now average resolution in x and y direction, respectively, rather than the average resolution, to determine the extent in lon/lat direction
      • now returning (lon_extent, lat_extent) instead of just lon_extent
    • Grid._grid_from_ds_adaptive:
      • Added fix for regional grids that include either meridian or antimeridian
      • Grids that are considered global in lon or lat direction, will now be defined within 0,360 / -180,180 or -90,90 degrees_east/north
    • Grid.to_netcdf:
      • Added fix for ds.encoding['unlimited_dims'] not being updated by xarray when dropping all time-dependent variables to write out solely the horizontal grid
      • Applying above fix for utf-8 encoding issues
    • Grid.extent_lat, Grid.extent_lon: added new attributes
    • Weights:
      • Will now use post_mask_source='domain_edge' setting when remapping a regional grid via nearest neighbour method, to avoid extrapolation beyond the original source domain (xESMF PR yet to be approved and merged)
  • Tests:
    • Added / modified tests for code changes
    • Added some tests to increase coverage
    • Reverted skipping or xfailing tests that used to fail with engine="h5netcdf"

Does this PR introduce a breaking change?:

  • clisops.core.regrid.Grid.detect_extent now returns the tuple (lon_extent, lat_extent) rather than just lon_extent
  • Grid.extent now carries the combined extent in lon and lat direction and no longer just the extent in lon direction. If the grid is considered global in extent of both, lon and lat coordinates, Grid.extent is set to "global" else, it is set to "regional". Grid.extent_lon and Grid.extent_lat allow accessing the extent in only lon and lat direction, respectively.

Other information:

I can update HISTORY.rst once all changes made by the PR are approved and potential feedback worked in.
There are currently two xesmf PRs open (#444 #445) that should be merged and a xesmf release triggered before merging this in.

- utils/common.py:
  - Updated require_module to accept an interval of unsupported versions
- utils/dataset_utils.py:
  - Added option to get_coord_by_type to suppress warning about a main variable
    not being identifiable
- utils/output_utils.py
  - Added functions 'fix_netcdf_attrs_encoding' and '_fix_str_encoding' to
    fix utf-8 encoding issues for global and variable attributes
- ops/base_operation.py:
  - Applying above fix to correct utf-8 encoding issues
- core/regrid.py:
  - xarray version warning correctly triggered for versions
    outside of compatible version interval
  - Grid.detect_extent:
    + determins now also extent in latitude
    + using now average resolution in x and y direction, respectively,
      rather than the average resolution, to determine the extent in lon/lat
      direction
    + now returning (lon_extent, lat_extent) instead of just lon_extent
  - Grid._grid_from_ds_adaptive:
    + Added fix for regional grids that include either meridian or antimeridian
  - Grid.to_netcdf:
    + Added fix for ds.encoding['unlimited_dims'] not being updated by xarray
      when dropping all time-dependent variables to write out solely the horizontal grid
    + Applying above fix for utf-8 encoding issues
  - Grid.extent_lat: added new attribute
  - Weights:
    + Will now use "post_mask_source='domain_edge'" setting when remapping a regional grid
      via nearest neighbour method, to avoid extrapolation beyond the original source domain
- Tests:
  - Added / modified tests for code changes
  - Reverted skipping or xfailing tests that used to fail with engine h5netcdf
@coveralls
Copy link

coveralls commented Aug 13, 2025

Pull Request Test Coverage Report for Build 18339990372

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 53 of 87 (60.92%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on regrid_updates_and_output_fixes at 76.498%

Changes Missing Coverage Covered Lines Changed/Added Lines %
clisops/utils/output_utils.py 21 23 91.3%
clisops/utils/common.py 0 5 0.0%
clisops/core/regrid.py 24 51 47.06%
Totals Coverage Status
Change from base Build 16940463822: 76.5%
Covered Lines: 2783
Relevant Lines: 3638

💛 - Coveralls

- Consistent distinction between extent, extent_lon, extent_lat
- Adaptive grids with global extent in lon/lat will not exceed
  -180,180 or 0,360 in lon / -90,90 in lat
- Modified tests accordingly
- Added tests for yet untested parts of the regrid code to boost coverage
@sol1105
Copy link
Contributor Author

sol1105 commented Sep 9, 2025

ToDo: Replace all .item calls with .data or .values (#451).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants