Skip to content

set gdal roles for well known tags#3570

Merged
pomadchin merged 4 commits intolocationtech:masterfrom
VitoTAP:geotiff_gdalmetadata
Apr 4, 2025
Merged

set gdal roles for well known tags#3570
pomadchin merged 4 commits intolocationtech:masterfrom
VitoTAP:geotiff_gdalmetadata

Conversation

@jdries
Copy link
Contributor

@jdries jdries commented Mar 20, 2025

#3496

Overview

Certain tags in GDAL are standardized, but this requires a 'role' attribute to be set.
In this PR, I take the approach of simply having a static list of such 'well-known' tags that seem to be part of the GDAL standard.

Checklist

  • ./CHANGELOG.md updated, if necessary. Link to the issue if closed, otherwise the PR.
  • Module Hierarchy updated, if necessary
  • docs guides update, if necessary
  • New user API has useful Scaladoc strings
  • Unit tests added for bug-fix or new feature

Notes

Caveat: if gdal invents new roles, we need to update the list.

Closes #3496

@jdries jdries marked this pull request as ready for review April 1, 2025 18:32
@jdries
Copy link
Contributor Author

jdries commented Apr 1, 2025

Ready for review, I found more tags that require the role attribute by looking at GDAL source itself:
https://github.com/OSGeo/gdal/blob/b8f2a16baa8fe866c8cd053d89120844d51ca94f/frmts/gtiff/gtiffdataset_write.cpp#L4208

We'd appreciate getting this merged, because we found out that fixing the tiff header afterwards breaks COG compatibility.

@pomadchin
Copy link
Member

pomadchin commented Apr 1, 2025

@jdries I'll take a look shortly and cut a release for you; there is also a tiny Proj4J release https://github.com/locationtech/proj4j/releases/tag/v1.4.0 that we'd needto include 👍

Thanks for making a PR!

@pomadchin
Copy link
Member

Also if you could take a look into the failing CI it would def help to get it merged faster!

@jdries
Copy link
Contributor Author

jdries commented Apr 2, 2025

CI ok!

Copy link
Member

@pomadchin pomadchin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍 I'll hopefully have time to review the rest of the PRs so we can get them merged before cutting a release.

@pomadchin pomadchin merged commit 7375fb0 into locationtech:master Apr 4, 2025
6 checks passed
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.

Geotiff Tags not fully compatible with GDAL

2 participants