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
Resource id refactor #2091
Merged
Merged
Resource id refactor #2091
Changes from 23 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
763c447
begin refactor
d-chambers 886e923
continue refactor of ResourceID
d-chambers 54bc75e
fix docstrings, use _debug_class_state context manager
d-chambers e566026
python 2 support
d-chambers 81f244a
small style fixes
d-chambers a18356f
add code and tests for pickle support
d-chambers 2f14d37
address krischer review
d-chambers 56f56be
add test for multithreaded catalog creation
d-chambers 1480ef4
start parent_key refactor
d-chambers c68311e
work on resource_id refactor
d-chambers d920cf9
continue refactor
d-chambers 02d3d45
fix resource_id doctest and flake issue in test
d-chambers 0d14ce0
cleanup test catalog creation
d-chambers 09b8491
update resource_id docs
d-chambers 8c878ab
refactor recursive object finder
d-chambers 6b36a38
exclude built-ins from unique id tests
d-chambers 5c94faa
implement custom function for resource_id recursion
d-chambers e61334c
py27 fix and pep8 issues
d-chambers 3bfea0a
fix CamelCase name in test_util_misc
d-chambers 18c9d05
rename resource_id import
d-chambers ba5aa11
Consistent sphinx references.
krischer 977120c
Removing the now obsolete rlock() decorator.
krischer 160d8dc
Also removing the _decorate_polyfill() function.
krischer 83bca69
de-indent _yield_resource_id_parent_attr
d-chambers 9c01708
change order of fun inputs in _yield_resource_id_parent_attr
d-chambers 207b856
re-enable skipped doctests
d-chambers 63e7817
add test for warnings not being raised
d-chambers aa26cfd
fix warning message
d-chambers 2fcbbad
Fixing an issue introduced via the rebasing.
krischer 5b87da9
Readding another test that got lost in the rebase.
krischer a9d923b
Some cleanup.
krischer 62e52dd
Adding changelog entry.
krischer be8a415
Scope the resource ids for each event reading plugin.
krischer ba9ecaf
Learning the alphabet.
krischer 33e0f8c
fix typo in warning
d-chambers dffd40d
add test for better warning
d-chambers fce9aa5
fix resource_id warning logic
d-chambers File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
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
Large diffs are not rendered by default.
Oops, something went wrong.
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
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
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
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
Oops, something went wrong.
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.
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.
How are things like the derived origin ids scoped after this function has been called?
_parent_key
would be set but_parent_id_tree
and things like this might not be set. But I just be misunderstanding some things.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.
Any object that is referenced by a resource_id should have the attribute
resource_id
. Lines 332-334 take care of this case, which would involve logging the identities of all the objects withresource_id
attributes into the proper place in the_parent_id_tree
. Then, resource ids that refer to other objects in the same event (but not their parent) such as thepick_id
of theArrival
object will find the correct object whenget_referred_object
is called because it was previously put into_parent_id_tree
. Because_object_id
on has been set toNone
it will not raise a warning. I added another test to specifically check for warnings being raised in this case.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.
👍 Thanks for the explanation. After spending some more time I think I now fully understand your changes and how everything works. Nice job!