r.in.gdal/r.external: store GDAL band metadata as semantic labels#7028
r.in.gdal/r.external: store GDAL band metadata as semantic labels#7028sakirr05 wants to merge 13 commits intoOSGeo:mainfrom
Conversation
…els (OSGeo#6227) - Query GDAL band description and BANDNAME; prefer description over name - Validate and store as raster semantic label via Rast_write_semantic_label - Skip silently when missing, invalid, or duplicate (multi-band) - Add tests for semantic label from band metadata (elevation.tif) - No naming or CLI changes; backward compatible
.github/workflows/print_versions.sh
Outdated
|
|
||
| # This will fail if the build failed. | ||
| grass --version | ||
| command -v grass >/dev/null 2>&1 && grass --version || echo "GRASS not installed yet" |
There was a problem hiding this comment.
How is this supposed to work? The script tries to run GRASS in the next line anyway.
You are just silencing the error message here in order to get it in the next line... As line 11 says, this will fail (and is supposed to fail) if the build fails...
There was a problem hiding this comment.
Thanks for pointing this out, I’ll revert the print_versions.sh change and all unrelated security/CI changes so the PR stays focused purely on the GDAL semantic label feature
ninsbl
left a comment
There was a problem hiding this comment.
Hm, ... I am not a C expert, but the implementation does not seem right to me. You re-implement existing library code in the tools and seem to duplicate code across r.external / r.in.gdal.
What is the motivation /reasoning for that?
|
BTW: Will your proposed changes change the behavior of r.in.gdal/r.external? Here I think esp. of map/file names of multiband data. |
|
@ninsbl One quick follow up- do you see this as implicit behavior that’s fine to keep as-is, or would you prefer it to be made explicit via a test or docs? |
ninsbl
left a comment
There was a problem hiding this comment.
The mechanics look in principle OK to me. That said, I am no C expert. So, I hope someone more knowledgeable could chime in!
What I noticed is that the functions get_gdal_band_semantic_label and set_semantic_label_from_gdal are duplicated in the tools (r.in.gdal, _r.external). That can probably be solved without function duplication ?
Also, not 100% sure if set_semantic_label_from_gdal is needed as a function at all...
Fixes #6227
This PR adds support for storing GDAL raster band metadata as GRASS raster
semantic labels during import with r.in.gdal and r.external.
When available, the GDAL band description (preferred) or band name is written
to the raster semantic label, allowing tools to understand band meaning
without changing raster names or band numbering.
Scope:
Example:
A raster with GDAL band descriptions "NDVI" and "EVI" will result in GRASS
rasters with semantic labels:
semantic_label=NDVI
semantic_label=EVI
Tests: