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

WIP: Add signatures for H5PT, H5LT #128

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jack-pappas
Copy link

I've created H5PT and H5LT classes with P/Invoke signatures for their respective HDF5 APIs (from hdf5-hl).

I've marked this as a work-in-progress (WIP) in the PR title for now, as I still need to implement some unit tests for the APIs.

@gheber
Copy link
Member

gheber commented Nov 20, 2017

@jack-pappas thanks for this pull request. The reason why we left H5PT & co. out of HDF.PInvoke is that they are not thread-safe. Maybe there's a better solution, be we might consider packaging/distributing them in separate assemblies. What do you think?

@jack-pappas
Copy link
Author

@gheber I'd be fine including the high-level APIs in a separate assembly (e.g. HDF.PInvoke.HighLevel), but I think that assembly/project should still live in this repo. Regarding whether to distribute it within the same NuGet package or not -- I think the argument could go both ways. I'd lean towards putting them in the same package though, since hdf5_hl.dll is already being included with the other native binaries (IIRC, but it's been a few months since I looked).

One thing that comes to mind though -- the low-level APIs aren't thread-safe either, unless HDF5 is compiled with thread-safety enabled. Even if that's how the binaries are being compiled for distribution as part of the NuGet packages (are they?), I think the wrappers here should still be agnostic to that -- users of this package may just want the HDF.PInvoke assembly to use with HDF5 binaries they've compiled themselves (for one reason or another). Given this library is meant to be a non-opinionated, zero-overhead wrapper around HDF5, maybe it's best not to design the wrapper around the thread-safety (or not) of the underlying APIs and leave that up to application developers to handle just as they would if e.g. writing a C++ library and linking it to hdf5.dll.

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

Successfully merging this pull request may close these issues.

2 participants