Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move HDF4 code to its own dispatch layer #748

Closed
edhartnett opened this issue Dec 27, 2017 · 5 comments · Fixed by #849
Closed

Move HDF4 code to its own dispatch layer #748

edhartnett opened this issue Dec 27, 2017 · 5 comments · Fixed by #849

Comments

@edhartnett
Copy link
Contributor

edhartnett commented Dec 27, 2017

When I added the HDF4 layer, the current dispatch system was not in use. So the HDF4 code just piggy-backs on the NetCDF-4 code in libsrt4, with code like this (from close_netcdf4_file()):

   /* Close hdf file. */
#ifdef USE_HDF4
   if (h5->hdf4)
   {
      if (SDend(h5->sdid))
         BAIL_QUIET(NC_EHDFERR);
   }
   else
#endif /* USE_HDF4 */
   {

Well this was a fun hack that has turned out to be very useful to some users. So it should be upgraded to it's own dispatch code. This would clean up the libsrc4 code, isolate the HDF4 code (removing all the ifdefs everywhere), and fit in better with the current architecture.

@DennisHeimbigner
Copy link
Collaborator

please do not attempt to move the hdf4 stuff. there are complexities.

@edhartnett
Copy link
Contributor Author

I was not planning on attempting it in the short term.

I would like to hear what the complexities are.

Actually, it looks rather straightforward to me.

@edhartnett
Copy link
Contributor Author

As discussed in our meeting, I will attempt this in a bit. I don't believe it will be hard.

In preparation I have removed some unneeded HDF4 checks in the def_var* functions. HDF4 is read only, so no need to worry about it in the def functions anyway.

@edhartnett
Copy link
Contributor Author

I did this yesterday, and it was not hard.

It's a nice cleanup for the libsrc4 directory, which is good, because there is already plenty of code and complexity there. Now the HDF4 code is all separate, in libhdf4, and there are no more ifdefs about HDF4 in the libsrc4 code.

It also demonstrates how a read-only format can be implement with just a few functions, relying on many of the libsrc4 (all the inq functions, for example). (A good step towards #177.)

I will put up a PR shortly.

@edhartnett
Copy link
Contributor Author

OK, looks like I need to wait a bit for Unidata netCDF to catch up on PRs before doing this merge. I will circle around to this again. Meanwhile the code is working in HPC NetCDF so won't go stale. ;-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants