Skip to content

Deprecate pygmt.io.load_dataarray, use xarray.load_dataarray instead - #3922

Merged
weiji14 merged 9 commits into
mainfrom
deprecate/load_dataarray
Apr 29, 2025
Merged

Deprecate pygmt.io.load_dataarray, use xarray.load_dataarray instead#3922
weiji14 merged 9 commits into
mainfrom
deprecate/load_dataarray

Conversation

@weiji14

@weiji14weiji14 commented Apr 19, 2025

Copy link
Copy Markdown
Member

Description of proposed changes

Deprecate usage of pygmt.io.load_dataarray for loading NetCDF and other grids, use xr.load_dataarray(..., engine="gmt") instead.

TODO:

  • Add FutureWarning
  • Update load_dataarray to xr.load_dataarray(..., engine="gmt", raster_kind="grid") internally
  • Move benchmark on test_io_load_dataarray to somewhere else
  • etc

Cherry-picked from #3919

Preview:

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash command is:

  • /format: automatically format and lint the code

Deprecate usage of `pygmt.io.load_dataarray` for loading NetCDF and other grids, use `xr.load_dataarray(..., engine="gmt")` instead.
@weiji14weiji14 added the deprecation Deprecating a feature label Apr 19, 2025
@weiji14weiji14 self-assigned this Apr 19, 2025
@weiji14weiji14 mentioned this pull request Apr 19, 2025
10 tasks
Specifically with arguments (..., engine="gmt", decode_kind="grid").
weiji14 added a commit that referenced this pull request Apr 19, 2025
@seisman

Copy link
Copy Markdown
Member

Perhaps we should also replace xr.open_dataarray with xr.load_dataarray in the tests?

@seismanseisman mentioned this pull request Apr 26, 2025
3 tasks
@seismanseisman added this to the 0.16.0 milestone Apr 26, 2025
@weiji14

Copy link
Copy Markdown
MemberAuthor

Perhaps we should also replace xr.open_dataarray with xr.load_dataarray in the tests?

I see you're doing some of that in #3643 already. Do you want to do it there, or in this PR?

@weiji14weiji14 added the run/benchmark Trigger the benchmark workflow in PRs label Apr 27, 2025
@codspeed-hq

codspeed-hqBot commented Apr 27, 2025

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #3922 will degrade performances by 85.18%

Comparing deprecate/load_dataarray (898f349) with main (f389e05)

Summary

❌ 2 regressions
✅ 102 untouched benchmarks
🆕 1 new benchmarks
⁉️ 1 dropped benchmarks

⚠️Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

BenchmarkBASEHEADChange
test_put_matrix_grid69 ms465.9 ms-85.18%
test_earth_relief_holes39.2 ms66 ms-40.6%
⁉️test_io_load_dataarray23.9 msN/AN/A
🆕test_xarray_backend_load_dataarrayN/A64.7 msN/A

@seisman

Copy link
Copy Markdown
Member

Perhaps we should also replace xr.open_dataarray with xr.load_dataarray in the tests?

I see you're doing some of that in #3643 already. Do you want to do it there, or in this PR?

Please do it in this PR.

@weiji14
weiji14 marked this pull request as ready for review April 29, 2025 02:46
@weiji14

Copy link
Copy Markdown
MemberAuthor

The gmt engine seems to be slower than the netcdf4 engine based on the codspeed report at https://codspeed.io/GenericMappingTools/pygmt/branches/deprecate%2Fload_dataarray (flame graphs seems to be missing, not sure when that happened). But at least we can remove the netcdf4 dependency soon in #3643 🚀

@weiji14
weiji14 merged commit ae82fd6 into mainApr 29, 2025
@weiji14
weiji14 deleted the deprecate/load_dataarray branch April 29, 2025 04:23
@weiji14weiji14 removed the run/benchmark Trigger the benchmark workflow in PRs label Apr 29, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecationDeprecating a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@weiji14@seisman