Skip to content
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

Fix wrap-around error in indexing for matrix-to-grid input #5947

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

PaulWessel
Copy link
Member

See #5294 for background. When desired subregion -Rw/e/s/n and a grid's internal region are off by 360 degrees relative to each other, we need to reconcile that shift before we compute column numbers since gridline vs pixel registration throws a wrench otherwise (we are off by one column). This PR implements the following:

  1. Adjusts the internal grid header wesn to be compatible with the desired sub region wesn before computing subset indices. The same fix also applies to gmtapi_import_cube.
  2. Updates the object's method variable when we switch from reference to duplicate (I don't think this caused any issues elsewhere but it is the right thing to).

CLI tests pass, and the pyGMT example returns the correct z-range. Please check that this PR works well for PyGMT, @meghanrjones.

When desired subregion -Rw/e/s/n and a grid's internal region are off by 360 degrees relative to each other, we need to reconcile that before we compute column numbers since gridline vs pixel throws a wrench otherwise.  This PR does

Adjust teh internal grid header wesn to be compatible with the desired sub region
Also updates the objects method when we switch from reference to duplicate (I do'nt think this caused any issues but it is the right thing to).
@PaulWessel PaulWessel added the bug Something isn't working label Nov 4, 2021
@PaulWessel PaulWessel added this to the 6.3.0 milestone Nov 4, 2021
@PaulWessel PaulWessel requested a review from maxrjones November 4, 2021 20:35
@PaulWessel PaulWessel self-assigned this Nov 4, 2021
Copy link
Member

@maxrjones maxrjones left a comment

Choose a reason for hiding this comment

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

Thanks, this fixes the result for pygmt/tests/test_grdimage.py::test_grdimage_shading_xarray. The only remaining issue here is this example: #5294 (comment), but that's unrelated to this PR.

@PaulWessel
Copy link
Member Author

Sorry, not quite sure what you mean (after reading that issue). What is not quite working?

@maxrjones
Copy link
Member

Sorry, not quite sure what you mean (after reading that issue). What is not quite working?

I'll post an explanation in that issue. This PR is good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants