Skip to content

Document interpolation restriction for grdtrack xarray input#1587

Closed
maxrjones wants to merge 4 commits intomainfrom
grdtrack-interpolant
Closed

Document interpolation restriction for grdtrack xarray input#1587
maxrjones wants to merge 4 commits intomainfrom
grdtrack-interpolant

Conversation

@maxrjones
Copy link
Member

Description of proposed changes

This PR documents the workaround to #1309 implemented in GenericMappingTools/gmt#5283.

Fixes #

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 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 commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@maxrjones maxrjones added documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog labels Oct 21, 2021
@maxrjones
Copy link
Member Author

/format

@maxrjones maxrjones changed the title Document bilinear interpolation restriction for grdtrack xarray input Document interpolation restriction for grdtrack xarray input Oct 21, 2021
@maxrjones maxrjones marked this pull request as draft October 21, 2021 16:43
Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like we need a unit test for this, checking the output of grdtrack when using a NetCDF file (default to bicubic) and xarray.DataArray input (default to bilinear).

Comment on lines +255 to +256
Note: Only bilinear or nearest-neighbor interpolation are currently
available when ``grid`` is an :class:`xarray.DataArray`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does b-spline work for xr.DataArray inputs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it will return bilinear interpolation results.

@maxrjones
Copy link
Member Author

Feels like we need a unit test for this, checking the output of grdtrack when using a NetCDF file (default to bicubic) and xarray.DataArray input (default to bilinear).

I'm not sure I follow what should be added. test_grdtrack_input_dataframe_and_ncfile already uses the default for ncfile (bicubic) and test_grdtrack_input_dataframe_and_dataarray uses the default for xarray.DataArray (bilinear).

@weiji14
Copy link
Member

weiji14 commented Oct 26, 2021

Feels like we need a unit test for this, checking the output of grdtrack when using a NetCDF file (default to bicubic) and xarray.DataArray input (default to bilinear).

I'm not sure I follow what should be added. test_grdtrack_input_dataframe_and_ncfile already uses the default for ncfile (bicubic) and test_grdtrack_input_dataframe_and_dataarray uses the default for xarray.DataArray (bilinear).

Ah ok, forgot that we had those comprehensive tests. Just ignore what I said then.

And actually, it looks like the change upstream in GenericMappingTools/gmt#5283 will be reverted according to GenericMappingTools/gmt#5893 (comment), so maybe we don't need this PR anymore?

@maxrjones maxrjones closed this Oct 27, 2021
@maxrjones maxrjones deleted the grdtrack-interpolant branch October 27, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation skip-changelog Skip adding Pull Request to changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants