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

CDF5 support in netCDF will be optional with next release. #713

Closed
WardF opened this issue Sep 21, 2017 · 14 comments
Closed

CDF5 support in netCDF will be optional with next release. #713

WardF opened this issue Sep 21, 2017 · 14 comments

Comments

@WardF
Copy link
Member

WardF commented Sep 21, 2017

See this netcdf-c issue for more information.

Starting with the upcoming 4.5.0-rc3 and moving forward, CDF5 support will be a configure time option. There are currently a couple of issues with it that are blocking the 4.5.0-rc3 release, and adding a way to disable support has eased the logistics of getting the release out while addressing the issue.

nc-config has a new option, nc-config --has-cdf5 which will return whether or not libnetcdf supports CDF5. Let me know if there's anything I can do to help make this easy for the python library to adapt to.

@jswhit
Copy link
Collaborator

jswhit commented Sep 21, 2017

Right now the python install script checks to see if netcdf.h has #define NC_FORMAT_64BIT_DATA in it - if so, CDF5 support is enabled. Will this approach still work in 4.5.0?

@WardF
Copy link
Member Author

WardF commented Sep 21, 2017

It will not. We also distribute netcdf_meta.h, I can add something in there along the lines of #define HAS_CDF5, let me take a look at it.

@jswhit
Copy link
Collaborator

jswhit commented Sep 25, 2017

Just tried running with v4.5.0-release-branch using --disable-cdf5 - it compiles fine, but as expected tst_cdf5.py fails. I don't think this is too big a deal though.

@WardF
Copy link
Member Author

WardF commented Sep 25, 2017

Sounds good; it's an expected failure of course, and if it isn't a problem for the netcdf4-python package, it isn't a problem for us. I wanted to say something ahead of time, however. I'm starting to prep the rc3 release today.

@jswhit
Copy link
Collaborator

jswhit commented Sep 25, 2017

If you can add HAS_CDF5 in netcdf_meta.h that would be helpful - that way I can disable the test if it is set.

@WardF
Copy link
Member Author

WardF commented Sep 26, 2017

Done; currently in the v4.5.0-release-branch but will be migrated out to master shortly.

@WardF WardF closed this as completed Sep 26, 2017
@ArchangeGabriel
Copy link
Contributor

@jswhit Can you disable the test if NetCDF is compiled without CDF5? This is the only test failure we see in our builds here, and we would like to avoid having to add a special case handling for this in our build process. ;)

@jswhit
Copy link
Collaborator

jswhit commented Oct 28, 2017

It's done it github master

@ArchangeGabriel
Copy link
Contributor

How is __has_cdf5_format__ determined? Because 1.3.1 still fails here:

======================================================================
ERROR: runTest (tst_cdf5.test_cdf5)
testing NETCDF3_64BIT_DATA format (CDF-5)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/build/python-netcdf4/src/netCDF4-1.3.1/test/tst_cdf5.py", line 19, in setUp
    v = nc.createVariable('var',np.uint8,'dim')
  File "netCDF4/_netCDF4.pyx", line 2437, in netCDF4._netCDF4.Dataset.createVariable
  File "netCDF4/_netCDF4.pyx", line 3389, in netCDF4._netCDF4.Variable.__init__
  File "netCDF4/_netCDF4.pyx", line 1638, in netCDF4._netCDF4._ensure_nc_success
RuntimeError: NetCDF: Not a valid data type or _FillValue type mismatch

----------------------------------------------------------------------

@jswhit
Copy link
Collaborator

jswhit commented Nov 1, 2017

if #define NC_HAS_CDF5 exists in netcdf_meta.h at build time

@ArchangeGabriel
Copy link
Contributor

Exists? Not the actual value? I’ve got this:

#define NC_HAS_CDF5      0  /*!< CDF5 support. */

@jswhit
Copy link
Collaborator

jswhit commented Nov 1, 2017

Oops. My understanding is that if CDF5 was not enabled, there would be no NC_HAS_CDF5 in nc_meta.h. I'll fix that.

@jswhit
Copy link
Collaborator

jswhit commented Nov 1, 2017

pull request #736

jswhit added a commit that referenced this issue Nov 1, 2017
fixed bug in detection of CDF5 library support (issue #713)
@jswhit
Copy link
Collaborator

jswhit commented Nov 1, 2017

pull request #736 merged, closing again

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

No branches or pull requests

3 participants