-
Notifications
You must be signed in to change notification settings - Fork 224
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
Add pygmt.load_dataarray function and refactor modules that return None or xarray.dataarray #1439
Conversation
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.
Looks great!
Co-authored-by: Wei Ji <[email protected]>
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.
Hi @meghanrjones, hoping that you can spend some time working on this PR soon (since there's a lot of open PRs that could benefit from a standardized function)! Based on the discussion above at #1439 (comment), here are some suggested changes. You'll also need to:
- Update
doc/api/index.rst
to include the newload_datagrid
function. My thought was to place it under 'Data Processing', but we could start a dedicated Input/output section in the API docs if you prefer, similar to https://geopandas.org/docs/reference/io.html. You'll also need to addfrom pygmt.io import load_datagrid
topygmt/__init__.py
.
Input/Output:
.. autosummary::
:toctree: generated
load_datagrid
- Find and replace
process_output_grid
toload_datagrid
across all thegrd*
functions. Remember to also includegrdproject
added recently in Wrap grdproject #1377 andload_earth_relief
atpygmt/pygmt/datasets/earth_relief.py
Lines 136 to 138 in ebbb0cc
with xr.open_dataarray(fname, engine="netcdf4") as dataarray: grid = dataarray.load() _ = grid.gmt # load GMTDataArray accessor information
Let me know if you need any help. I'm happy to push some changes directly to this branch too if you're ok with working on this together.
Thanks for the ping @weiji14. I will prioritize this PR tomorrow (8/31) or am fine with you pushing changes if you post a message first. |
Co-authored-by: Wei Ji <[email protected]>
@weiji14, thanks for your detailed comment. I named the function |
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.
Looks a clean solution to me!
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.
@weiji14, thanks for your detailed comment. I named the function
load_dataarray
rather thanload_datagrid
to be more specific about the output type. Of course, you can comment if you disagree with this decision.
Ok with renaming this to load_dataarray
. It's not a commonly used function I think, so hopefully the xarray
folks don't mind 😸 Other than that, only a few more minor suggestions on the docstrings.
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
I would prefer to wait to merge this until after #1458 so that I can resolve the merge conflict rather than causing one for that PR. |
Patches missing code coverage of pygmt.load_dataarray function wrapped in #1439. Checks that a NetCDF grid can be read with GMTDataArrayAccessor information loaded, and that load_dataarray fails when the cache argument is used.
…ne or xarray.dataarray (GenericMappingTools#1439) Add new function pygmt.load_datarray Use pygmt.load_dataarrary for modules that return grids Add documentation Co-authored-by: Wei Ji <[email protected]>
…gTools#1518) Patches missing code coverage of pygmt.load_dataarray function wrapped in GenericMappingTools#1439. Checks that a NetCDF grid can be read with GMTDataArrayAccessor information loaded, and that load_dataarray fails when the cache argument is used.
Description of proposed changes
This PR adds a new pygmt/io.py module that holds process_output_grid, which returns an xarray.DataArray or None depending on whether outgrid matches tmpfile.name.
It replaces duplicate code with the new function.
Fixes #1400
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version