Skip to content

Conversation

@willirath
Copy link
Contributor

Copy link
Member

@benbovy benbovy left a comment

Choose a reason for hiding this comment

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

Looks great!

We'll need to add a few more tests (could be done later):

  • test the XoakAccessor index property
  • test non-transformed coordinate values (perhaps with other data than geo coordinates)
  • test NN lookup tolerance
  • check that the order of coordinate labels in the returned selection is preserved (I checked this locally simply by inspecting the results and it works)
  • shallow test for the DataArray.xoak accessor

@willirath
Copy link
Contributor Author

I'd prefer merging this soon (maybe after CI is added), because then, the general repo structure is fixed and we can work on different aspects of the ToDo's you listed in separate PRs.

@benbovy
Copy link
Member

benbovy commented Jun 27, 2020

Ok! Can I merge this now? :-)

@willirath
Copy link
Contributor Author

Yes, go ahead. :)

@benbovy benbovy merged commit 00028d0 into master Jun 27, 2020
@willirath willirath deleted the setup branch June 27, 2020 16:53
This was referenced Jun 28, 2020
@benbovy benbovy mentioned this pull request Dec 15, 2020
Closed
6 tasks
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.

3 participants