Skip to content

Apply scalings in api_get_record_matrix #5799

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

Merged
merged 1 commit into from
Sep 27, 2021
Merged

Apply scalings in api_get_record_matrix #5799

merged 1 commit into from
Sep 27, 2021

Conversation

maxrjones
Copy link
Member

Description of proposed changes

Similar to #5291, but for matrices. Unlike that PR, I did not do anything about checking whether GMT_IS_REFERENCE | GMT_IS_DUPLICATE is needed.

All GMT and PyGMT tests pass, except one which is wrong (xref GenericMappingTools/pygmt#1539). I did not test GMT.jl.

Addresses GenericMappingTools/pygmt#1313

Reminders

  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@joa-quim
Copy link
Member

This is the type of things I keep saying we should avoid. If there is something to convert we should have a different for loop to deal with it, instead of adding more and more if conditions inside the same loop.

Not particular to this case, though.

@maxrjones
Copy link
Member Author

This is the type of things I keep saying we should avoid. If there is something to convert we should have a different for loop to deal with it, instead of adding more and more if conditions inside the same loop.

Not particular to this case, though.

The code could be refactored separately, but in this PR I am simply fixing a bug using the current structure.

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.

3 participants