gjoseph92 / stackstac

Turn a STAC catalog into a dask-based xarray
https://stackstac.readthedocs.io
MIT License
232 stars 49 forks source link

Add dtype validation when rescale=True #209

Closed RSchueder closed 10 months ago

RSchueder commented 1 year ago

This MR adds validation of the inputs to stackstac.stack to ensure that the output dtype is compatible when rescale=True. Rescaling requires that the output dtype is floating-point.

RSchueder commented 1 year ago

@gjoseph92 I would like to assign you as a reviewer. I am hoping this mention enables this as per the solution here (I am more of a GitLab person myself).

RSchueder commented 12 months ago

@gjoseph92 I am not sure we can ever have a (scale, offset) of dtype other than float because of:

https://github.com/RSchueder/stackstac/blob/20f7dbbfc5e7cd29e3b9faec20e8b16aea64a2d8/stackstac/prepare.py#L30

If this is true, isn't actually checking the value a bit redundant since we know it will be a float?

RSchueder commented 12 months ago

Okay, so even if the (scale, offset) is always technically (float, float), we only apply the value if it != (1,0). Therefore, I now only do the check on scale != 1 and offset != 0. Hopefully this makes sense.

gjoseph92 commented 11 months ago

Sorry for the delay @RSchueder. I'd still like to move the check around here https://github.com/gjoseph92/stackstac/blob/7836a363b2609216890b0e20e289f8ff71cf320d/stackstac/prepare.py#L158

That way, you get the error immediately, instead of having to wait until compute.

RSchueder commented 10 months ago

Now worries @gjoseph92!

Gotcha, I didn't want to modify the signature of prepare_items but I have now implemented the change in prepare_items.