xoak icon indicating copy to clipboard operation
xoak copied to clipboard

Expose indexers

Open willirath opened this issue 4 years ago • 6 comments

This splits the .sel method into two steps (getting the positional indexers), and actually doing the selection.

The aim of this is to provide an easy way of obtaining the indices for use in downstream applications like horizontal interpolation etc.

  • [x] Add method for directly querying the index (.query)
  • [x] Tests for the new logic (unchanged coverage is not a good metric here)
  • ~~[ ] Add query keyword arguments and pass them to the underlying index~~
  • [x] Add examples:
    • [x] Using the .query() method for calculating distances between target and selected points
    • ~~[ ] Using the indexers for interpolation (on a structured grid)~~
    • ~~[ ] Using the indexers for a two-step process with step one just getting the nodes for a polyline and step two querying all intermediate points.~~

Test on binder: https://mybinder.org/v2/gh/willirath/xoak/expose-indexers?filepath=doc%2Fexamples

willirath avatar Dec 20 '20 11:12 willirath

@benbovy This is not ready yet and mostly meant for discussing if and how we'd expose indexers. But feel free to have a look and leave comments as soon as you see fit.

willirath avatar Dec 20 '20 11:12 willirath

The aim of this is to provide an easy way of obtaining the indices for use in downstream applications like horizontal interpolation etc.

I agree that it's valuable to provide users with some API so that they can do something with the indices.

Actually, I think that we could rename get_indexer to query and generalize so that it more-or-less matches scipy and sklearn indexes' query methods, here returning xarray object(s) instead of numpy array(s).

A possible signature would look like:

ds.xoak.query(
    indexers: Dict,
    return_distance: bool = True,
    query_kwargs: Optional[Dict] = None,
    **indexer_kwargs
)

(or alternatively expand query_kwargs instead of indexer_kwargs, or expand none of them).

Likewise, we could later extend the API with other common methods like query_radius, etc., with optional support by xoak's index adapters...

benbovy avatar Dec 20 '20 16:12 benbovy

I'll reduce this to just providing a query method for now. Passing arbitrary kwargs adds a lot of complexity because we loose an easy way of knowing about the structure of the data returned by then indexer, then.

willirath avatar Mar 01 '21 10:03 willirath

Should we wait for this PR to be merged before moving the repository to xarray-contrib?

benbovy avatar Mar 09 '21 15:03 benbovy

Do as you like. I can easily reopen this PR once it's moved.

willirath avatar Mar 09 '21 16:03 willirath

Doc build fails with unrelated "Command killed due to excessive memory consumption". The rest is (finally) good for a review.

I've decided to skip the more elaborate examples for now.

willirath avatar Aug 01 '21 11:08 willirath