observingClouds / xbitinfo

Python wrapper of BitInformation.jl to easily compress xarray datasets based on their information content
https://xbitinfo.readthedocs.io
MIT License
52 stars 20 forks source link

Check for NaNs and raise warning #200

Open ayoubft opened 1 year ago

ayoubft commented 1 year ago

As discussed with @observingClouds and @milankl, and raised in this BitInformation.jl issue: the elements in an array should be checked whether they are finite and raise a warning if they are not. As having many NaNs in the dataset may yield unexpected results.

A check could be using is_null() from xarray like:

ds.is_null().sum()

Or perhaps by iterating through a dataset and issuing a warning upon encountering the first NaN value.

observingClouds commented 1 year ago

ds.is_null().any() is probably the way to go. The function is embarrassingly parallel. This shouldn't have much influence on the performance. I probably would call this when we need to load the data anyway.

ambiguousphoton commented 11 months ago

If this issue is still open please assign it to me, I would like to work on it.

observingClouds commented 11 months ago

Hi @ambiguousphoton, Thank you for your interest in this project. I just assigned you to this issue. I'm looking forward to review your contribution and am happy to help. Cheers!

Shaheer-Ahmd commented 6 months ago

@observingClouds I would love to implement this. Can you please guide me about where is this check needed in the codebase?

observingClouds commented 5 months ago

@Shaheer-Ahmd sry for the late response. If you are still interested, the implementation could be added before https://github.com/observingClouds/xbitinfo/blob/7341975a65f97267ca4c1c124deb28f6694b380a/xbitinfo/xbitinfo.py#L180. Please note though that we also want to have tests implemented for this feature.

observingClouds commented 5 months ago

unassigned @ambiguousphoton due to inactivity

observingClouds commented 4 months ago

This issue will be fixed by https://github.com/observingClouds/xbitinfo/pull/260