Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set_ndv does not update nodatas in array #286

Closed
rhugonnet opened this issue Aug 29, 2022 · 2 comments · Fixed by #309
Closed

set_ndv does not update nodatas in array #286

rhugonnet opened this issue Aug 29, 2022 · 2 comments · Fixed by #309
Labels
bug Something isn't working

Comments

@rhugonnet
Copy link
Member

rhugonnet commented Aug 29, 2022

During set_ndv, the array is not updated with previous nodata.

Steps to reproduce the behavior:

r = gu.Raster(landsat_b4_path)
r.data.ravel()[0:100]=255
r.set_ndv(255)
np.max(r.data)
Out[22]: 254
np.max(r.data.data)
Out[23]: 255
r.set_ndv(254, update_array=True)
np.max(r.data.data)
Out[25]: 255

All the 255 no data values should have been updated to 254 by the set_ndv(..., update_array=True), which is also the default parameter.

@adehecq
Copy link
Member

adehecq commented Aug 29, 2022

I don't think it's a bug. Maybe we should clarify in the docstring, but looking at the code, it seems like the behavior of set_ndv is meant to be:

  • update/set self.nodata
  • if update_array=True, unmask old masked values, mask values currently set to nodata.

This is exactly what is done in your example, i.e. the 255 values are masked, then unmasked.
We just have to agree on what is the typical case in which one would use set_ndv. I don't think the example that you show is something that one would typically want to do, why set a nodata value and then change it later?

It also raises the broader question. With masked_arrays, the masked values are actually not being modified. If one runs self.set_mask, self.data.mask is modified but not self.data.data. This can be an issue with certain rasterio's functions actually that do not use the mask.

@adehecq adehecq changed the title set_mask does not update nodatas in array set_ndv does not update nodatas in array Aug 29, 2022
@adehecq
Copy link
Member

adehecq commented Aug 31, 2022

Ok, after discussions, I changed my mind.
We need to update the behavior so that it is possible to update the nodata value while keeping the same mask (useful if a certain nodata value is not appropriate, e.g. because it is used).
Also, the current functionality of update_array=True does not make sense to me. Masked pixels can contain any value, so when updating the array, we should make sure that currently masked values are set to the previous nodata value.
In summary, we need to :

  • update update_array=True so that the previously masked pixels are filled with the previous nodata value (e.g. to reverse an accident in a set_ndv call)
  • add an option preserve_mask set to True by default, which keeps the previous mask. I'm unsure what to do if values are set to the new nodata. Maybe raise a warning?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants