Remove deprecated Cython IF, DEF
#1277
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This helps remove lots of the deprecated Cython IF/DEF statements
We only need to know if we have any parallel support at compile time, and not whether it's netcdf4 or pnetcdf flavour
To avoid the last deprecated compile-time `IF`, we instead separate the two branches of the conditional into different files and choose which one to include at build time.
We also need to explicitly cast the comm/info arguments to the correct type
|
Thanks for working on this! Since this is a pretty big change, and the IF/DEF support in cython is not going away anytime soon, I'd like to leave this open for discussion for a while before merging (after the next release perhaps). |
|
Sure, that sounds reasonable! I'm happy to break it into smaller PRs if you prefer |
|
I don't see any reason to split it up, but we'll see what others think. |
|
Now that 1.6.5 is out, this can be merged as soon as it is ready. @ZedThree please bump the version number to 1.7.0 and add a Changelog entry. |
* master: include fix to pyproject.toml for 3.12 fix missing part of previous commit fix for pkg_resources missing on 3.12 version 1.6.5 release remove 'oversubscribe' option from mpirun fix typo add python 3.12, remove python 3.7
No longer needed
|
I think this is ready now |
jswhit
added a commit
that referenced
this pull request
Apr 17, 2024
Fix bug in set_collective added in #1277
ocefpaf
pushed a commit
to ocefpaf/netcdf4-python
that referenced
this pull request
Apr 26, 2024
Fix bug in set_collective added in commit f64ee26 that cause access always be in collective mode. Independent access mode is not tested so it passed unnoticed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1269
The main trick I've used here is having a new C header
netcdf-compat.hwhich does two things:setup.pyThis allows us to eliminate most of the
DEFstatements and compile-timeIFconditionals. We can also include everything unconditionally from this compatibility header innetCDF4.pxi. The use of the functions is still guarded at runtime, and I've flipped most of these guards to raise exceptions as soon as possible. The stubs all returnNC_EINVALso if for some reason a guard is missed or bypassed, they'll still raise an error and not silently fail.I needed to use a different technique for the conditional
cimportof mpi4py. For this, I moved the two branches into separate files,[no_]parallel_support_imports.pxi.in. The build system then chooses which one to copy toparallel_support_imports.pxiwhich we can then unconditionally include in the.pyxfile.