mdbartos / pysheds

:earth_americas: Simple and fast watershed delineation in python.
GNU General Public License v3.0
710 stars 195 forks source link

replace usage of np.bool with builtin type #212

Open avkoehl opened 1 year ago

avkoehl commented 1 year ago

For this package to work with numpy > 1.20.0 I needed to replace all the deprecated np.bool dtypes with the builtin bool type.

https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations

The deprecation warning: DeprecationWarning:np.boolis a deprecated alias for the builtinbool. To silence this warning, useboolby itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, usenp.bool_here. Deprecated in NumPy 1.20; for more details and guidance: https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations

Not sure if the builtin type or np.bool_ is more appropriate for this code.

avkoehl commented 1 year ago

Worth noting that this issue of deprecated types also prevents numba from importing. Which leads me to #201

mdbartos commented 1 year ago

Thanks for submitting this. To clarify: you cannot use the numba-accelerated functions because the deprecated types prevent sgrid.py from importing?

avkoehl commented 1 year ago

Thanks for submitting this. To clarify: you cannot use the numba-accelerated functions because the deprecated types prevent sgrid.py from importing?

Yeah, that is right.

Here I create a poetry environment with pysheds 0.3.3. Screenshot 2023-02-06 at 9 35 50 AM These are the dependencies it installs. See numpy 1.24.2

On import of 'pysheds.grid' it seems to use pgrid: Screenshot 2023-02-06 at 9 38 32 AM

And if I try to import numba:

Screenshot 2023-02-06 at 9 34 59 AM

mdbartos commented 1 year ago

Thank you. From what I can tell, it seems like there may be an issue with your numba installation, and that is what is preventing sgrid.py from importing.

The non-numba functions in pgrid.py have not been supported for a while and are kept in the code for compatibility reasons. I can still merge this request though.

avkoehl commented 1 year ago

That makes sense. Good to know that sgrid is preferred. I will use those methods then.

In case others run into this, my fix for numba not importing was to return to an older version of numpy that still contained the np.long type.