Add a subject reference to PoseEstimation and deprecate it on Skeleton - #66
Open
h-mayorquin wants to merge 1 commit into
Open
Add a subject reference to PoseEstimation and deprecate it on Skeleton#66h-mayorquin wants to merge 1 commit into
PoseEstimation and deprecate it on Skeleton#66h-mayorquin wants to merge 1 commit into
Conversation
A Skeleton describes a morphology and may be shared, so it cannot name the animal once a file describes several. PoseEstimation gains an optional subject region over any DynamicTable, so it points at the SubjectsTable of ndx-multisubjects when that is present without ndx-pose depending on it. The Skeleton link warns on write and stays silent on read, following rly#33 and rly#57.
|
Note that there is a related blocker upstream with ndx-multisubjects: NeurodataWithoutBorders/pynwb#2219 We've been discussing with Oliver and Ryan for a while how to go about a fix |
This file contains hidden or 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
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 have been thinking about adding multi-subject support for the pose estimation interfaces in neuroconv. Currently the subject specifier in this extension is on the
Skeleton(see #51), but I think it does not work very well here because two subjects of the session might have exactly the same skeleton. In this PR I am moving the subject specifier from theSkeleton(which is now deprecated) to thePoseEstimationcontainer, which is where I think it makes sense.The alternative here is the "parent-container-as-subject-identity" pattern of the current
ndx-multisubjects, which does not clash with this, though it does mean the identity depends on where an object sits rather than on the object itself, so anything that resolves objects by name at a known level has to learn the nesting. Meanwhile, enabling this means that we can link to theSubjectsTableof that extension, or allow users to roll their own outside of that scope. What do you think?The deprecation follows the convention already in the repository: the
subjectargument inSkeletonraises aDeprecationWarningwhen a user sets it, reading an existing file that has it stays silent through_in_construct_mode, and the spec doc carries theDEPRECATED.prefix pointing at the replacement. That is whatnodesandedgesgot in #33 and whatoriginal_videos,labeled_videosanddimensionsgot in #57, so this lands beside them rather than being stricter than its neighbours. Say if you would rather it refused outright.This should close #51.