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

Make reduce_to_pole work for arbitrary dimension names #509

Merged

Conversation

zaarcvon
Copy link
Contributor

@zaarcvon zaarcvon commented Jun 13, 2024

Fix bug in reduction_to_pole that made it to work only if the dimensions of the passed grid were northing and easting. Now the grid can have arbitrary dimension names. Add tests for these changes.

Relevant issues/PRs:

Fixes #439 bug issue: the reduction_to_pole filter produces an error when grid coordinate names are not "northing" and "easting".

Now, it works regardless of grid coordinate names.

@leouieda
Copy link
Member

@zaarcvon thank you for the fix! Would you mind adding a quick test in that runs the reduction to the pole with a grid with dims that have another name? It could be a modification of this test https://github.com/fatiando/harmonica/blob/main/harmonica/tests/test_transformations.py#L615 that renames the dims of the test grid before doing RTP. Would be worth checking that the test fails with the code in main first.

@leouieda leouieda changed the title fix bug #439 with reduction_to_pole filter error Fix bug in reduction to the pole requiring dims "easting" and "northing" Jun 14, 2024
@zaarcvon
Copy link
Contributor Author

Hi, @leouieda! I included a test function. I hope everything is done correctly. This is my first experience working with pytest

@zaarcvon zaarcvon force-pushed the fix-reduction_to_pole-coord-name-bug branch from d0c713c to 8978bbf Compare June 14, 2024 19:25
@zaarcvon
Copy link
Contributor Author

Hi, @leouieda . I see that some checks are failing. I understand one related to style and will fix it, but it's not clear what the code style/format check is really about.

@santisoler
Copy link
Member

Hi @zaarcvon. The two check that are failing are due to code style. The format one is easy to solve, we just need to run black on the project. You can do it by running make format in your local repo (https://github.com/fatiando/community/blob/main/CONTRIBUTING.md#code-style). The style one is telling us that flake8 is raising some warnings. You can check them out by clicking in Details, next to the failing check. In this particular case, flake8 is complaining about trailing whitespaces (whitespaces left after the last character of certain lines). When you run make format, black will take care of those as well.

@santisoler santisoler added this to the v0.7.0 milestone Jun 25, 2024
@santisoler
Copy link
Member

Hi @zaarcvon! Today @YagoMCastro fixed the code style issues in your PR, so now we are ready to merge this. Thanks a lot for the contribution! 🚀

@santisoler santisoler changed the title Fix bug in reduction to the pole requiring dims "easting" and "northing" Make reduce_to_pole work for arbitrary dimension names Aug 12, 2024
@santisoler santisoler merged commit 07a5af3 into fatiando:main Aug 12, 2024
19 checks passed
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.

Filter reduction_to_pole works only with "northing", "easting" coordinate names
4 participants