Skip to content
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

r3.out.netcdf: Use current major version in history attribute #2342

Merged
merged 5 commits into from
Feb 15, 2023

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented Apr 28, 2022

r3.out.netcdf creates a netCDF file with attribute history which contains software. This removes the hard-coded software version information and uses the current major version instead for the history text. It uses fairly general macros to encapsulate stringification to avoid complex code needed to use snprintf.

The documentation contains the output, so the version should be there, but I removed it from the examples to avoid the need for updates (making the documentation little less precise).

@wenzeslaus wenzeslaus added this to the 8.4.0 milestone Apr 28, 2022
@neteler neteler added the raster Related to raster data processing label Aug 28, 2022
@wenzeslaus wenzeslaus force-pushed the no-major-version-in-netcdf branch from 4c5697c to b358313 Compare February 10, 2023 19:11
@wenzeslaus wenzeslaus marked this pull request as ready for review February 10, 2023 19:12
@wenzeslaus
Copy link
Member Author

This is ready for review and would be good to get merged before v8.3 because it removes some outdated references to v7 in the code.

The question is, is removing the GRASS GIS version number from the metadata desired?

@neteler
Copy link
Member

neteler commented Feb 11, 2023

The question is, is removing the GRASS GIS version number from the metadata desired?

As it is actually HISTORY_TEXT being set, I'd suggest to replace the hardcoded version with the respective VERSION variable containing the GRASS GIS version used to write out the file.

@wenzeslaus
Copy link
Member Author

It now uses GRASS_VERSION_MAJOR. The string manipulation code is somewhat complicated (necessary?).

I did not update the documentation, so the PR is removing the version number from there right now. I'm not sure how to make it simple to maintain while keeping it absolutely precise.

I could also use GRASS_VERSION_STRING which is already a string, but it contains year ("@GRASS_VERSION_NUMBER@ (@GRASS_VERSION_DATE@)"). Alternatively, we could also define GRASS_VERSION_NUMBER in C in version.h. In any case, this would make the documentation harder to update if we opt for including the version number there.

@wenzeslaus wenzeslaus changed the title r3.out.netcdf: Remove major version from attribute r3.out.netcdf: Always use current major version in attribute Feb 13, 2023
@wenzeslaus wenzeslaus force-pushed the no-major-version-in-netcdf branch from 6d8702f to 7babe1f Compare February 14, 2023 16:01
@nilason
Copy link
Contributor

nilason commented Feb 14, 2023

You will need to clang-format this.

r3.out.netcdf creates a netCDF file with attribute history which contains software. This drops the software version information.
@wenzeslaus wenzeslaus force-pushed the no-major-version-in-netcdf branch from e16e4c0 to 747fb1c Compare February 14, 2023 19:00
@wenzeslaus wenzeslaus added the enhancement New feature or request label Feb 15, 2023
@wenzeslaus wenzeslaus changed the title r3.out.netcdf: Always use current major version in attribute r3.out.netcdf: Always use current major version in history attribute Feb 15, 2023
@wenzeslaus wenzeslaus changed the title r3.out.netcdf: Always use current major version in history attribute r3.out.netcdf: Use current major version in history attribute Feb 15, 2023
@wenzeslaus
Copy link
Member Author

Thanks for feedback @neteler and @nilason. I'm ready to merge this.

Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@nilason nilason added the C Related code is in C label Feb 15, 2023
@wenzeslaus wenzeslaus merged commit 4f5477c into OSGeo:main Feb 15, 2023
@wenzeslaus wenzeslaus deleted the no-major-version-in-netcdf branch February 15, 2023 20:14
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
…2342)

r3.out.netcdf creates a netCDF file with attribute history which contains software. This removes the hard-coded software version information and uses the current major version instead for the history text. It uses fairly general macros to encapsulate stringification to avoid complex code needed to use snprintf.

The documentation contains the output, so the version should be there, but I removed it from the examples to avoid the need for updates (making the documentation little less precise).
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
…2342)

r3.out.netcdf creates a netCDF file with attribute history which contains software. This removes the hard-coded software version information and uses the current major version instead for the history text. It uses fairly general macros to encapsulate stringification to avoid complex code needed to use snprintf.

The documentation contains the output, so the version should be there, but I removed it from the examples to avoid the need for updates (making the documentation little less precise).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C enhancement New feature or request raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants