Removing NIX C++ bindings [dev branch] #276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The removal of the bindings lets us clean out the Mixins so we can rewrite the class in a much cleaner fashion. The File.open() function now simply calls the File() constructor.
ProxyLists are going to be removed but for now, I need to duplicate this definition to avoid an import cycle.
h5group.py and h5dataset.py are now separate and work as a backend module. Future changes should isolate the backend from the frontend, so file.py would not import h5py
Option is deprecated
More efficient version of ProxyList
Before it was assumed that if the group property is None that the underlying underlying HDF5 group did not exist, since instantiating a group sets the property in such cases. With the new Container class, a long living H5Group object but be created that does not have a group prop set but it ends up being created in another place (presumably, from a _create_obj() method).
LinkContainer requires reference to parent Block containers. Container and LinkContainer __init__ docstrings added. LinkContainer reimplements certain functions due to difference in indexing links in the backend.
With parent backend (H5Group) reference, it can't instantiate new NIX objects properly when loading from the backend.
This is not standard behaviour but it's how it worked for RefProxyList. We might change the behaviour in the future, but for now, the API shouldn't change.
Objects retrieved from LinkContainers would have a '_parent' reference to the linking parent (e.g., Group) instead of the location where they were created (e.g., Block). This is fixed now. The objects are created with a reference to the 'itemstore' parent.
Currently fails because LinkContainer doesn't handle Sources properly.
Subclasses LinkContainer
Rebasing against master left some things in a broken state. The following fixups are applied: - Import os in tests - Use testfilename class member in tests - Use new alias range dimension creation method - Fix for reading data from integer arrays with float polynomial coefficients - Delete leftover pycore files
|
So this is huge and probably impossible to read. I can probably squash and rebase and slice it down to smaller batches build and test successfully. I'm going to write some cross-compatibility tests starting today. Perhaps we can review those first and then base this PR and future ones off the new tests. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm starting a new dev branch to keep track of the progress of the removal of the bindings from NIXPY and the flattening of the class hierarchy. The bindings removal started a while ago and since we all agree on going ahead with it now, I thought I'd start a dev branch where I can PR changes as I work on them, to avoid having a huge PR with all the changes in the end.
The current PR is the state of the branch as it was a few months ago. The bindings are completely gone here, but it still needs a lot of work and cleanup. There's more work that I've put into it more recently, but I think this stage is pretty good for an initial review.
It's also rebased against master. I'll regularly keep the dev branch up to date with master to make the transition easier in the end.
I know it's already quite big, so take your time with the review.