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

Error in SwathDefinition html representation if lon/lat arrays are dask arrays #609

Closed
BENR0 opened this issue Jul 23, 2024 · 5 comments · Fixed by #610
Closed

Error in SwathDefinition html representation if lon/lat arrays are dask arrays #609

BENR0 opened this issue Jul 23, 2024 · 5 comments · Fixed by #610

Comments

@BENR0
Copy link
Contributor

BENR0 commented Jul 23, 2024

Code Sample, a minimal, complete, and verifiable piece of code

This was discovered while using the li_l2_nc reader but affects all SwathDefinitions where the lat/lon DataArrays don't have a resolution set in the attributes.

Problem description

If lon/lat data in the SwathDefinition are dask arrays the html representation throws a type error due to None Type not having a round method because no resolution can be found in the attributes.

Especially in the Li point data case this problem might not be easily solveable.

Expected Output

A html represetation of the SwathDefinition.

Actual Result, Traceback if applicable

    260     # use resolution from lat/lons dataarray attributes -> are these always set? -> Maybe try/except?
    261     area_name = f"{lon_attrs.get('sensor')} swath"
--> 262     resolution_str = "/".join([str(round(x.get("resolution"), 1)) for x in [lat_attrs, lon_attrs]])
    263     area_units = "m"
    265 height, width = area.lons.shape

TypeError: type NoneType doesn't define __round__ method

Versions of Python, package at hand and relevant dependencies

@djhoese
Copy link
Member

djhoese commented Jul 23, 2024

My guess is this isn't because they are dask but rather a dask array with unknown shape. This comes from doing operations that filter data and drop values rather than replace them with NaNs as we usually do. Usually in satpy we use whatever the dask method is to force the computation of the shape.

@BENR0
Copy link
Contributor Author

BENR0 commented Jul 23, 2024

The Problem is actually that the xarrays don't have a resolution set in the attributes which for Li point data is difficult I guess?
I already anticipated something like this that's why the comment is in the code. When I opened the bug report I thought that this can easily be solved by also using the estimation calculation form the ndarray case. But as it turns out in the Li point data case this is not possible because the lat/lot arrays are not 2 dimensional. Even when working around this a little bit there is still the problem later on that the SwathDefinition does not have a width/height. I always thought that the SwathDefinition was a little "brittle" and might need some more work. With Li point data this came sooner than I expected 😬.

@pnuu
Copy link
Member

pnuu commented Jul 23, 2024

LI data is not point data per se, so adding the resolution would make sense in that regard.

@djhoese
Copy link
Member

djhoese commented Jul 23, 2024

Ah sorry, I misread the error message on my phone. SwathDefinitions have no guarantees about the shape of the data or even the geographic contiguous-ness of the locations. And resolution in the DataArray attrs is only a Satpy thing so not guaranteed for sure.

@BENR0
Copy link
Contributor Author

BENR0 commented Jul 25, 2024

LI data is not point data per se, so adding the resolution would make sense in that regard.

@pnuu Yes true but in the non gridded case it is kind of handled like point data I think. At least it is very difficult to calculate a resolution from non 2D sparse lat/lon arrays I guess. I think what @djhoese said about the SwathDefinition not having no guarantees about the shape of the data as well as the resolution as an attribute in the DataArray makes me think that at this point the problem at hand is not easily solvable. So for the time being I think it would be good to at least catch that case and give some kind of message in order to not break the html representation completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants