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

Add enable-legacy-macros option. #3029

Closed
WardF opened this issue Sep 19, 2024 · 1 comment · Fixed by #3030
Closed

Add enable-legacy-macros option. #3029

WardF opened this issue Sep 19, 2024 · 1 comment · Fixed by #3030
Assignees

Comments

@WardF
Copy link
Member

WardF commented Sep 19, 2024

As observed in #3017 and #3018, the change introduced in #2911 (discussed in #2858), re: refactoring the macro _FillValue to NC_FillValue has caused some issues for projects; it is easily fixed, but it is a problem when the issue potentially requires changing a projects code. While it is tempting to say it is the problem of downstream projects, we should provide a mechanism to ameliorate the inconvenience and also recognize that for larger projects, there are often processes in place which make 'just change your code' more difficult than it sounds.

I'll add an option, 'enable-unsafe-macros', which will be off by default but, when true, will define _FillValue in parallel with NC_FillValue.

@WardF WardF self-assigned this Sep 19, 2024
@edwardhartnett
Copy link
Contributor

edwardhartnett commented Sep 19, 2024

I don't think this should be an option, it should be something that always happens.

_FillValue is part of the API, it's been used for decades. Provide NC_FillValue but also leave _FillValue defined. If end-users have a problem with it, they can undef it in their netcdf.h file.

Removing _FillValue will just cause everything to break. It's a direct violation of our commitment to backward compatibility of code. Whatever harm is done by leaving _FillValue in, pales with the harm of breaking everyone's code on a netcdf-c update. (If you want people to stop updating netcdf-c and just stay with 4.9.2 for the next 20 years, this is a good way to achieve it.)

If you search through Russ' old presentations you will find the "Certificate of Backwards Compatibility" he used to display, in which he called the backward compatibility "sacrosanct". Users depended on that guarantee and followed documented best practices using the _FillValue macro.

If you must have an option, then make it default to including _FillValue, because that's what everyone needs to do.

@WardF WardF changed the title Add enable-unsafe-macros option. Add enable-legacy-macros option. Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants