-
Notifications
You must be signed in to change notification settings - Fork 95
File-access property lists API (+MPI / direct VFD support) #31
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
Conversation
SQUASH ME
(Failures on both AppVeyor and Travis are reasonable, will be fixed tonight; kind of proves that a proper test suite is not just a waste of time). Would appreciate if anyone could take at least a very brief look at this since it's quite a lot of stuff :) (@pmarks, @Enet4?) The next step (and the last step before 0.3.0 release) would be refactoring a major portion of |
@aldanor very cool! agreed that dropping the bindgen requirement is a nice win. Things look reasonable, but I have no experience with fapls and fancy IO modes, so I'm not really qualified to comment. |
Thanks for comment @pmarks :) I guess the most user-visible change after this gets merged and subsequent changes to This would make things more verbose, but more logical and organized as well (hopefully not too verbose). So, the current let file = FileBuilder::new()
.userblock(1024) // belongs to FCPL H5P API
.driver("core") // belongs to FAPL H5P API
.filebacked(false) // belongs to core-driver portion of FAPL H5P API
.mode("x") // belongs to H5F file API
.open("foo.h5")?; // H5F file API as well would be more of something like this (preliminary sketch): let fapl = plist::FileAccess::build().core().finish()?; // all of H5P FCPL stuff
let fcpl = plist::FileCreate::build().userblock(1024).finish()?; // all of H5P FAPL stuff
let file = File::create_excl_with(&fcpl, &fapl, "foo.h5")?; // all of H5F file stuff To make the latter less verbose, (Note that most of the time you'd just simply use |
Alas, my experience with FAPL and related contents is also quite limited. But if I can provide at least some assurance, I did not notice anything reprehensible in the changes, and all tests pass on my machine. I also agree with the idea of improving the File API. I already started using the crate for my own tools, and there are probably some interactions that could be improved. |
@Enet4 thanks for checking it out.
Would love to hear if there’s anything aside from what’s been already discussed (reworking file creating / opening api to be more like stdlib and integrating fcpl/ fapl in a more hdf5-like manner). |
Upgrade to proc_macro_error2.
plist::FileAccess
) with its own builder; almost all properties from HDF5 C API are supported, including those version-gated or feature-gated (like mpio/direct).bindgen
from build dependencies (yay!); parseH5pubconf.h
header manually with regex. This is a pretty big win since we no longer depend on clang-sys/libclang which may cause version compatibility problems with other crates at build time.feature = "mpio"
in both hdf5-rs and libhdf5-sys crates; only available whenh5_have_parallel
is detected. Added missing FAPL/DXPL bindings.h5_have_direct
is detected.H5FD_*_init()
calls are now mutex-locked (no more segfaults due to that).hdf5-rs
one level up, so it's now actually the root crate which also happens to be a workspace (instead of being one of the workspace crates) -- this allows running tests with--features
enabled.This took a bit longer than expected (planned to just add simple wrappers for FAPL), but a lot of stuff got fixed in the process including the build system, plus we now support all imaginable HDF5 backends and a convenient framework for building and testing other property list types.
A few open questions:
Currently, particular property lists are accessible like
plist::FileAccess
. While that makes sense when spelled like so,FileAccess
on its own doesn't - it's not obvious that it's a property list.FileAccessPropertyList
? (the base type is calledPropertyList
) That sounds too long though?...FileAccessPL
? That looks a bit cryptic... (less than "fapl" but still)Currently, there's two version of each property getter:
#[doc(hidden)] pub fn get_foo(&self) -> Result<T>
pub fn foo(&self) -> T
Wondering if it's ok?
The main reason is to allow easy access to properties when you're sure your objects are fine and valid (which would be the case 99.999% of time). So, like, instead of doing
plist.get_userblock().unwrap()
you can just do aplist.userblock()
.This gets a little weird with more complicated types, e.g. if a struct is returned we now need to have something legible to return in case the getter failed for some reason. That being said,
get_*()
getters are still available if the user wants full control over possible errors (typically, the errors with getters would occur only if the plist became invalid, e.g. if you closed a file handle that the plist was attached to, or something like that).Oh, and btw... property lists are now nicely printable: