makepath / xarray-spatial

Raster-based Spatial Analytics for Python
https://xarray-spatial.readthedocs.io/
MIT License
805 stars 81 forks source link

Multispectral tools: convert data to float32 dtype before doing calculations #755

Closed thuydotm closed 1 year ago

thuydotm commented 1 year ago

Fixes #726.

TODO:

codecov-commenter commented 1 year ago

Codecov Report

Base: 79.88% // Head: 79.88% // No change to project coverage :thumbsup:

Coverage data is based on head (bb04f35) compared to base (af42544). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #755 +/- ## ======================================= Coverage 79.88% 79.88% ======================================= Files 19 19 Lines 4170 4170 ======================================= Hits 3331 3331 Misses 839 839 ``` | [Impacted Files](https://codecov.io/gh/makepath/xarray-spatial/pull/755?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=makepath) | Coverage Δ | | |---|---|---| | [xrspatial/multispectral.py](https://codecov.io/gh/makepath/xarray-spatial/pull/755?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=makepath#diff-eHJzcGF0aWFsL211bHRpc3BlY3RyYWwucHk=) | `69.76% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=makepath). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=makepath)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

thuydotm commented 1 year ago

Almost our tools return data of dtype f32. Although the calculations on f32 can cause some minor numerical error if the original data is of f64, we save the result in f32 I think those errors are acceptable. In case of issue #726, converting from uint16 to floating point dtype (f32 specifically) is necessary to make sure result of a calculation does not exceed the bounds of unsigned int data type.

brendancol commented 1 year ago

@thuydotm Can you follow up with a PR with documentation of how these types are handled?

https://github.com/makepath/xarray-spatial/issues/766