-
Notifications
You must be signed in to change notification settings - Fork 6
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
More robust lon/lat initialization for imagestack #47
base: master
Are you sure you want to change the base?
Conversation
The previous initialization of lon/lat could break if the grid had an unusually row/col arrangements or unusual shape. np.unique() is a bit slower but more robust. The sort() is probably redundant, but just wanted to be on the save side.
removed redundant sort
Just some general thoughts: The whole image part of pynetcf is used very little and not very well developed. I'm not sure if we should even allow the link of @sebhahn should decide this PR and the future direction of the image part depending on the needs of TUW. |
@sebhahn as discussed, the Also added lon/lat check (if lon/lat is broken, it will read the wrong gpis) |
I think this could cause some issues. Before you where using the Using |
The way I see it is that both approaches are not 100% fail safe. The question is if imagestack is for 2d grids only or can be extended to non-2d grids? |
There are several options for coordinate systems that we could support (http://cfconventions.org/Data/cf-conventions/cf-conventions-1.7/cf-conventions.html#coordinate-system) . A non-2d grid |
I can remember we had this discussion some time ago and I wasn't too happy about the naming (ArrayStack and ImageStack). Coming back to this two classes it still confuses me. There seems to me not 1-to-1 relationship to CF coordinate systems, but rather an abstract layer to combine the reader with a grid, right? @cpaulik could you please shortly summarize the distinct differences and use cases of ArrayStack and ImageStack? I have to admit that I haven't been using them a lot, because I'm not an Image user. I hope clarifying the meaning helps to understand if we need two classes here or just a general one. |
The goal of this package is to write CF conform netCDF files. I think we should completely rewrite the The What is the current use case for the |
@Lmoesinger could you please elaborate on your use case? |
I´m using it to convert time series (stored as GriddedNcContiguousRaggedTs, divided into multiple files by a cellgrid) to to a stack of 2d matrices by iterating over each gpi. All time series have the same time indexes. I know the dates beforehand as they are regularly spaced between a known start- and enddate. I`m only writing one time series per GPI. Basically it is very similar to Also note that it wasn't my goal to make this working with grids that don't have a shape - rather my goal was to make it work with grids that have an unusual shape. |
I think we can discuss this tomorrow before lunch @sebhahn . @Lmoesinger if you also work at the TU then we might also meet tomorrow. |
Sure, at what time and where? |
The current lon/lat initialization of
ImageStack
can break if the grid has an unusual row/col arrangement or an unusual shape. E.g. if you create an imagestack with this grid all grid points will have the same lat and lon:The proposed change uses
np.unique()
to get the lat/lon values, which works independent of the grids orientation.I ran the tests with the changes and broke none of them, but it still might now screw up some other grid.