Open adybbroe opened 9 months ago
As mentioned in your pull request we can't assume xarray is available in Pyresample. That is standard in Satpy, but not pyresample. If I'm understanding your PR correctly, your swath DataArrays have xarray dimensions ("x", "y")
? Is that correct? In that case, I have two possible solutions that I see:
("y", "x")
. While technically it shouldn't matter, I'm a little scared that this is only one piece of the Satpy/Pyresample puzzle that you're finding an issue with, but that we make the assumption of ("y", "x")
in many places.Ok, thanks for the comments and suggestions. And, yes, the dimensions are ("x", "y")
which is non-standard, I know (see below).
Concerning option 2: Yes, but actually in this case it is the data format which is "wrong", or non-standard. So, for now I rather keep the reader as it is, and wait for the agency (ESA) to fix the format. They have acknowledged the issue at least.
But, as the reader returns an xarray DataArray I thought it would be appropriate to actually try use that information, rather than make assumption on the data layout.
Having said that, I also like your option 1. So, would you prefer:
Concerning option 2: Yes, but actually in this case it is the data format which is "wrong", or non-standard. So, for now I rather keep the reader as it is, and wait for the agency (ESA) to fix the format. They have acknowledged the issue at least.
I don't agree with this, but I'm not doing the coding so I can accept it. Everyone is admitting it is "wrong" (or at least unexpected) and the solution should be a simple .T
on the array. Anyway...
Having said that, I also like your option 1. So, would you prefer:
- Keep the suggestion above when the data is an xarray DataArray, and then improve the code for Numpy cases as you propose in your option 1? Or
- Skip using the xarray capability and improve as in your option 1, which will will work in all cases?
I prefer 2. It (at least theoretically) should only improve the accuracy of this method which is at best a guess. I'd prefer to avoid Xarray workarounds as much as possible, especially when the alternative is "make this functionality work better", but I could also be missing some complexity in this task so I could be convinced otherwise.
I agree that option 2 is preferable, and that the reader should be adapted such that the order of dimensions is (y, x)
. This is the simplest solution that allows other parts of pytroll to stick to the assumption that dimensions have this order, without the need to complicate the code everywhere where that assumption is currently made.
Code Sample, a minimal, complete, and verifiable piece of code
The problem is well illustrated by reading and trying to remap the AWS tesdata:
See the comments in the Satpy PR here: https://github.com/pytroll/satpy/pull/2565
In the method to determine the optimal radius of influence to be used when reampping data (https://github.com/pytroll/pyresample/blob/6a8afc0085e0b4269f00991ab79fe1b3766bb817/pyresample/geometry.py#L664) the assumption is that the first dimension on the longitude array dscribes the scanlines/rows of the (e.g. channel) dataset.
The specific code lines that should be adapted in case that
self.lons
is anxarray.DataArray
is this part I believe:I would propose something like this instead:
Problem description
[this should also explain why the current behaviour is a problem and why the expected output is a better solution.]
Expected Output
Actual Result, Traceback if applicable
Versions of Python, package at hand and relevant dependencies