-
Notifications
You must be signed in to change notification settings - Fork 85
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
added band_as_variable option to open_rasterio #592
Conversation
Thanks for taking a stab at this @clausmichele 👍 The design in #296 would be to load the band data in separate variables so you can do this: xds = rioxarray.open_rasterio('./results/S2_L2A_data_mod.tiff',band_as_variable=True)
xds["B02"] This enables storing the description, nodata values, etc separately in the attributes of each variable. An example implementation is here: https://github.com/bopen/xarray-gdal/blob/main/xarray_gdal/xarray_plugin.py |
# If we have a Dataset with variables and each variable has already 'grid_mapping: spatial_ref' | ||
# in the variable attrs, doing the next steps would add another 'spatial_ref' as a | ||
# coordinate and will break the functionality of xarray.to_nectdf() | ||
# TODO: Probably we would need to check if the transform is already available in each band? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The transform should be the same for each band.
rio_crs = riods.crs or result.rio.crs | ||
if rio_crs: | ||
result.rio.write_crs(rio_crs, inplace=True) | ||
# If we have a Dataset with variables and each variable has already 'grid_mapping: spatial_ref' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend not adding the CRS to each variable as you did previously and instead add it to the dataset here.
for attr, value in result.attrs.items() | ||
if not attr.startswith(f"{result.name}#") | ||
} | ||
# remove duplicate tags for Dataset, otherwise there are issues opening the netCDF file in QGIS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting. Mind adding a separate function called _clean_attrs
and moving the duplicate cleanup there?
output_is_dataset = True | ||
data_vars = {} | ||
for i in riods.indexes: | ||
data_var_attrs = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if band specific tags should be added here?
@clausmichele thanks for working on this. I added some comments and will try to do a more thorough review later. |
Okay, this got me down a rabbit hole to a different implementation to preserve the lazy loading (see #600). Want to give it a try and see what you think? |
First draft of a possible implementation of
read_variable_as_bands
.@snowman2 what do you think? Is it too naive as an approach?
docs/history.rst
for all changes anddocs/rioxarray.rst
for new API