Skip to content

Conversation

@aaronzedwick
Copy link
Member

@aaronzedwick aaronzedwick commented Sep 26, 2024

Closes #963 #924

Overview

Adds tests to get a more complete code coverage. Takes reused code from nearest_neighbor and inverse_distance and creates one general function for them to call, this should cut down on code and make it easier to create new remapping functions. Updates documentation so the remapping functions are consistent with each other. Also fixes a bug I found when trying to switch from cartesian or spherical after performing one remap.

PR Checklist

General

  • An issue is linked created and linked
  • Add appropriate labels
  • Filled out Overview and Expected Usage (if applicable) sections

Testing

  • Adequate tests are created if there is new functionality
  • Tests cover all possible logical paths in your function
  • Tests are not too basic (such as simply calling a function and nothing else)

Documentation

  • Docstrings have been added to all new functions
  • Docstrings have updated with any function changes
  • Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst
  • User functions have been added to docs/user_api/index.rst

@aaronzedwick aaronzedwick marked this pull request as draft September 26, 2024 14:17
@aaronzedwick aaronzedwick changed the title Remapping Code Coverage, Modularize Reused Code, and Documentation Updates DRAFT: Remapping Code Coverage, Modularize Reused Code, and Documentation Updates Sep 26, 2024
@aaronzedwick aaronzedwick marked this pull request as ready for review October 1, 2024 14:43
@aaronzedwick aaronzedwick changed the title DRAFT: Remapping Code Coverage, Modularize Reused Code, and Documentation Updates Remapping Code Coverage, Modularize Reused Code, and Documentation Updates Oct 1, 2024
Copy link
Collaborator

@ahijevyc ahijevyc left a comment

Choose a reason for hiding this comment

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

Switch from ux.open_dataset to ux.open_grid where possible, replacing destination_dataset.uxgrid with destination_grid afterwards.

Can we avoid hard-coding reconstruct=True when getting the Ball Tree?
This probably needs to wait for Grid.get_ball_tree to be enhanced.

Copy link
Collaborator

@ahijevyc ahijevyc left a comment

Choose a reason for hiding this comment

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

Great switch to open_grid. Thanks for looking into necessity of reconstruct=True

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.

Improve testing coverage for uxarray.remap

5 participants