1733 address behavior ophys nwb issues #1760
Merged
Conversation
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
This commit removes fields that already exist as part of the pyNWB "Subject" class from the custom behavior ophys subject extension.
Previously dff and fluorescence traces were being stored as ROIs x timepoints Numpy arrays. This commit implements pyNWB best practice by storing the data with the time dimension first.
This is a combination of the following 3 commits: 1) Simplify custom behavior ophys pyNWB extension Previously, the OphysBehaviorMetadata extension included a number of fields that already exist in base pyNWB classes. This commit moves "emission_lambda", "excitation_lambda", "indicator", "location", "session_start_time", and "imaging_rate" fields so that they are stored in pyNWB classes like ImagingPlane and OpticalChannel. This greatly simplifies the OphysBehaviorMetadata extension definition. 2) Add more detailed "doc" description to pyNWB extension fields 3) Address pyNWB NWBFile.modules deprecation warning Trying to access pyNWB processing modules via the "NWBFile.modules" attribute is now deprecated in favor of "NWBFile.processing". This commit implements the favored method for accessing pyNWB processing modules from an NWBFile object.
Previously, ROIs were added to the behavior ophys NWB file incorrectly. ROIs all ended up starting at the 0,0 corner of the whole imaging space. This was caused by feeding the "LIMS formatted ROI" (List[List[bool]]) through the allensdk.brain_observatory.roi_masks.create_roi_mask function twice instead of only once. This happened once in allensdk.internal.api.ophys_lims_api.OphysLimsApi get_cell_specimen_table() method and yet again in the allensdk.brain_observatory.nwb.__init__ add_cell_specimen_table() function. Now, only the OphysLimsApi get_cell_specimen_table() is the only place where LIMS formatted ROIs are deserialized to the allensdk.brain_observatory.roi_masks.RoiMask class. A CellSpecimenTable schema discrepancy was also fixed. 'x' and 'y' fields should be Int not Float based on what the behavior_ophys_write_nwb_strategy from LIMS is passing in.
To better align with best practice guidelines: https://www.nwb.org/best-practices/
This commit adds experiment descriptions to be havior ophys NWB files. These descriptions can be determined based on the behavior ophys "session type" string. Example: "OPHYS_1_images_A" is the first stage of the visual behavior experiments "OPHYS_6_images_A" is the last stage of the visual behavior experiments
Contributor
Author
|
@matchings @dougollerenshaw Let me know if you'd like any changes for the wording of experiment descriptions: ac7fde7 |
kschelonka
reviewed
Oct 21, 2020
Contributor
|
Yes, probably LIMS would be better since that is our source of truth. But I
think a config file would be acceptable to start with, so let's not let
perfect be the enemy of good. :)
…On Wed, Oct 21, 2020 at 9:48 AM Nicholas Mei ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In allensdk/brain_observatory/nwb/ndx-aibs-behavior-ophys.extension.yaml
<#1760 (comment)>
:
> @@ -3,112 +3,84 @@ groups:
neurodata_type_inc: LabMetaData
doc: Metadata for behavior or behavior + ophys task parameters
attributes:
+ - name: task
Yes, I can move the experiment descriptions to a separate config-style
file. The pyNWB behavior-ophys extension would probably not be the right
place to put that information though :/
Another thought I had was, would it make sense to store experiment
descriptions in LIMS and have it pass that info down? That would probably
be the 'kosher' way to do it, but seemed to me like it would involve a
protracted process.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#1760 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIFEJBSLT4G6CRL3JMUBBGLSL4GG3ANCNFSM4SZ7ILDQ>
.
|
Codecov Report
@@ Coverage Diff @@
## rc/2.4.0 #1760 +/- ##
============================================
+ Coverage 36.16% 36.30% +0.13%
============================================
Files 346 346
Lines 33819 33859 +40
============================================
+ Hits 12232 12293 +61
+ Misses 21587 21566 -21
Continue to review full report at Codecov.
|
kschelonka
reviewed
Oct 23, 2020
allensdk/brain_observatory/behavior/behavior_ophys_api/behavior_ophys_nwb_api.py
Outdated
Show resolved
Hide resolved
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.
Overview:
This PR contains commits to address pyNWB best practice recommendations for behavior ophys NWB files.
Addresses:
Addresses issue: #1733
Changes:
Validation:
Checklist
Allen Institute Contribution Guidelines
Numpy Standards
appropriate
rc/x.y.z as its merge target
Notes: