-
Notifications
You must be signed in to change notification settings - Fork 747
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
Include hdf.hdf5lib.* java packages #1327
Conversation
We currently end up compiling the
|
We may want to have |
I think it's fine having our project (re)compile the source. It makes sure we have all the dependencies in there. And we can make those dependencies "optional" so they are not forced upon the user.
Sure, I guess that should be fine. |
Great. Is there anything else that needs to be done? I am mainly pausing to test this out, but that could also be done from the snapshots. I'm also evaluating if there are any elements from the SIS JHDF5 bindings that may be useful: https://sissource.ethz.ch/sispub/jhdf5/-/tree/master/source/c |
It looks fine, we can merge this, yes, but let's do it after it's been tested a bit locally, yes. |
Why do half of the platforms use cmake and the other half use autotools? HDF Group seems to moving towards retiring autotools: |
CMake wasn't available previously. We can switch to CMake, of course.
|
hdf5/cppbuild.sh
Outdated
# Copyright 2007 - 2018 ETH Zuerich, CISD and SIS | ||
# Apache License, Version 2.0 | ||
# https://sissource.ethz.ch/sispub/jhdf5 | ||
cp ../../../src/main/c/jhdf5/*.c java/src/jni/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't add them to this repository. Download them just like HDF5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The files from JHDF5 need significant modification since they were last used with HDF 1.10.
I could start a branch on my fork and pull from there. Would that be better?
https://github.com/JaneliaSciComp/jhdf5
In the meantime, could we run the CI so I can see if there are any cross platform issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just rewrite it in Java then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might be the easiest route. I'll revert recent changes
fcdec66
to
0775e1e
Compare
I'm willing to move ahead with this. I'll try the full Java approach to addressing the JHDF5 legacy. |
To be clear, the future Java work lies outside of the scope of this PR. I'm done here unless there is something else to fix. Let me know if you there are any other issues to address. |
It may be useful to enable SZIP compression via libaec https://gitlab.dkrz.de/k202009/libaec . The conda-forge build of HDF5 recently added support for SZIP compression via libaec in December 2022: I plan to work on this in a separate branch based on this branch combined with changing all the builds to use cmake. |
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright (C) 2018-2022 Samuel Audet | |||
* Copyright (C) 2023 Mark Kittisopikul |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Thank you.
This includes the
hdf5_java.{dll, so, dylib}
as well as the following Java packages:hdf.hdf5lib
hdf.hdf5lib.callbacks
hdf.hdf5lib.exceptions
hdf.hdf5lib.structs