-
Notifications
You must be signed in to change notification settings - Fork 26
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
File references #462
Merged
Merged
File references #462
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 creator method is meant to be used by parent classes. I had originally made it "private" to imply it's for internal library use only. In the future, we might fold the creation functionality into the __init__ function along with the instantiation of objects from a file.
All Entities are initialised with and keep a reference to the File object. Containers keep the same reference to initialise objects that it instantiates from the file. Non-Entity objects that also require this reference: - Feature - Dimension objects
Testing in container for convenience (has all objects) and because instantiating objects through containers is where an object might lose references to parent objects.
Type and Definition changes on Block.
- Rename 'time_auto_update' to 'auto_update_timestamp' - Property and constructor argument are the same - Enabled by default: Updating timestamps should be the default. The overhead is probably minimal (perhaps even negligible) and should only be disabled for performance when required.
Instead of separating based on property, separate based on option state.
This pull request introduces 2 alerts when merging 2fc194e into 9b63d54 - view on LGTM.com new alerts:
|
- Removed hacky parent traversal for getting root objects like the top level metadata container. - Removed H5Group utility functions that are now obsolete. - Containers that need to do a search from the root object to delete objects now use the file reference.
jgrewe
approved these changes
May 6, 2020
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.
lgtm
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.
Every object that subclasses Entity now holds a reference to the nix.File object that they belong to. This reference is the same object for all. The reference is useful for a few reasons but more recently it becomes necessary for being able to read the
auto_update_timestamps
option, which is a session option only (it's not stored in the file) and is only available through the original File object.Fixes #460
Changes in this PR:
auto_update_timestamps
. When enabled, it updates theupdated_at
timestamp for an object when one of its properties are changed.NOTE: The NIX compatibility tests will fail until we fix ASCII reading (see G-Node/nix#817).