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

Improve docstrings of Raster.value_at_coords and Raster.interp_points, or merge the two methods. #395

Open
erikmannerfelt opened this issue Sep 1, 2023 · 1 comment
Labels
documentation Improvements or additions to documentation enhancement Feature improvement or request

Comments

@erikmannerfelt
Copy link
Contributor

Is your feature request related to a problem? Please describe.
See GlacioHack/xdem#423 for the general context of this issue. Basically, the docstrings of Raster.value_at_coords and Raster.interp_points are so similar that it's very difficult to figure out which one to use.

@rhugonnet described it nicely:

I have a deadline today, will look in more details tomorrow.

Quick answer on your last question:

* The `value_at_coords` function extracts the closest point value to the coords or its average (any type of reductor function) within a square window of a certain size (doesn't require to load the entire Raster).

* The `interp_points` function performs a regular grid interpolation (nearest, linear, cubic) to extract the values at the coords.

More details in the examples: https://geoutils.readthedocs.io/en/stable/analysis_examples/index.html#point-extraction But, yes, we definitely need to change the names and improve the docstrings of those functions...

Describe the solution you'd like
I see two potential solutions for this:

  1. Improve the docstrings to make them stand out more from each other. They could also refer to each other to be even clearer on the different use cases
  2. Merge the two functions and make the interp_points extra functionality into a keyword argument.
@erikmannerfelt erikmannerfelt added documentation Improvements or additions to documentation enhancement Feature improvement or request labels Sep 1, 2023
@adehecq
Copy link
Member

adehecq commented Nov 8, 2023

Another point:
The docstring of Raster.interp_point regarding argument input_latlon is not clear enough. It's unclear whether the input coordinates should be given as (lon, lat) or (lat, lon). It currently says "Whether the input is in latlon, unregarding of Raster CRS".
According to the test suite, it looks like it is (lat, lon). This would need to be checked and clarified in the doc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement Feature improvement or request
Projects
Development

No branches or pull requests

2 participants