search

CNES / pangeo-pyinterp

Python library for optimized interpolation.
https://pangeo-pyinterp.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
117 stars 17 forks source link

within functionality flipped for IDW #7

Closed bolliger32 closed 3 years ago

bolliger32 commented 3 years ago

Describe the bug within kwarg seems to be flipped for inverse_distance_weighting and in example notebooks. See https://github.com/CNES/pangeo-pyinterp/blob/47520d6585ea7bcfda7c57fd3c32692e90c10633/src/pyinterp/core/include/pyinterp/detail/geometry/rtree.hpp#L184

Also, in https://github.com/CNES/pangeo-pyinterp/blob/master/notebooks/auto_examples/ex_unstructured.ipynb there are comments saying that within = False implies "# Extrapolation is forbidden". In reality, this should imply that extrapolation is allowed.

Same with https://github.com/CNES/pangeo-pyinterp/blob/master/notebooks/auto_examples/pangeo_unstructured_grid.ipynb

Expected behavior within=True should imply that extrapolation is forbidden, which matches the docstrings for the functions and matches the behavior for radial_basis_function and query

Pyinterp/Numpy/Python version information Output from

import sys, numpy, pyinterp.version
print(pyinterp.version.release(True))
print(numpy.__version__)
print(sys.version)

I get an AttributeError b/c pyinterp has no attribute version. But pyinterp.__version__ yields 0.6.1

fbriol commented 3 years ago

Thanks a lot for your feedback!

You are right. The bug is in the C++ code where I flipped the method that should be used with the keyword within. The documentation is correct, but the behavior is not.

I have corrected the notebooks to be consistent with this fix.

And I also fixed the template for the bug report. I changed the access to the library version, but not the template.

bolliger32 commented 3 years ago

thanks! And thanks for all your work on this tool - it's super useful!