Skip to content

Added suppport for geotiff colormap() colorinterp to be loaded into attrs #3136

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

Closed
wants to merge 2 commits into from
Closed

Added suppport for geotiff colormap() colorinterp to be loaded into attrs #3136

wants to merge 2 commits into from

Conversation

alando46
Copy link

@alando46 alando46 commented Jul 17, 2019

This is tangentially related to the to_geotiff() discussion.

We're using intake-xarray to read a year's worth of Sentinel-2 Scene Classification Layers into a DataArray, doing some processing, and then writing the output to a single band GeoTIFF. When writing the output, we'd like to preserve the colormap of the original underlying data. xarray-intake uses xarray to read the dataset, so thought it might be useful to add support in open_rasterio() to read and load src.colormap(1) and src.colorinterp into the attributes of the DataArray. My thinking is that if in the future the community ends up wanting to add a to_geotiff() method, reading the src colormap and colorinterp might be useful.

Before adding tests, I thought I'd submit the PR to see what others thought about adding this functionality. If there's support for this, I'm happy to add any needed tests or documentation.

  • Tests added
  • Fully documented, including whats-new.rst for all changes and api.rst for new API

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #3136 into master will decrease coverage by 0.94%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #3136      +/-   ##
==========================================
- Coverage   95.99%   95.05%   -0.95%     
==========================================
  Files          63       63              
  Lines       12799    12803       +4     
==========================================
- Hits        12287    12170     -117     
- Misses        512      633     +121

@dcherian
Copy link
Contributor

I think we'd like to load all possible attrs and make them accessible if it's easy to do so.

Is there a way to do that without having to explicitly add each one?

@alando46
Copy link
Author

@dcherian that makes sense to me and shouldn't be difficult to do. I basically just tried to follow the existing design pattern for the other attributes.

@keewis
Copy link
Collaborator

keewis commented Apr 13, 2021

related to #4697: now that we consider deprecating xr.open_rasterio in favor of xr.open_dataset(..., engine="rasterio") (implemented by rioxarray), should this be moved there?

@snowman2
Copy link
Contributor

Related: corteva/rioxarray#191

weiji14 added a commit to weiji14/rioxarray that referenced this pull request Oct 1, 2021
Cherry-picked from pydata/xarray#3136

Co-Authored-By: Alando Ballantyne <[email protected]>
@dcherian
Copy link
Contributor

dcherian commented Apr 7, 2022

Should be fixed in rioxarray since we've now deprecated open_raserio.

@dcherian dcherian closed this Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants