Skip to content

Conversation

@pickles-bread-and-butter
Copy link
Contributor

Overview:

This pull request addresses a failure in NWB writing where Behavior Ophys experiments do not have the hard coded stimulus_name key available. This has now been rectified by adding in a small function that checks for the existance of equivalent keys in the stimulus table dataframe.

Validation:

I've included test cases to test for the existance of the keys in the provided keys, ie: checking for a column name within a set of column names. These cases cover the cases where it finds the eccephys key or the behavior ophys key. It also covers the case where it finds no keys and the case where it finds more than one key.

I have also run the BehaviorOphysNWBAPI.save() function that calls this function and it proceeds past this point, correctly grabbing the names of the stimuli in the stimuli table.

Keys for the name of stimulus are different between eceephys and behavior ophys.
This adds in a small function to get the key represented in each data type and
raise an exception if one key is not found. This was added to allow ophys
behavior experiments to write to nwb files.
I updated the function to be more generic and removed specific case verbaige. I also fixed the failing tests on Linux and Mac. The regex for the tests was returning an error on Linux and Mac.
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #1619 into address-behavior-ophys-NWB-failures will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                           Coverage Diff                           @@
##           address-behavior-ophys-NWB-failures    #1619      +/-   ##
=======================================================================
+ Coverage                                35.58%   35.60%   +0.01%     
=======================================================================
  Files                                      344      345       +1     
  Lines                                    33568    33577       +9     
=======================================================================
+ Hits                                     11946    11955       +9     
  Misses                                   21622    21622              
Impacted Files Coverage Δ
allensdk/brain_observatory/nwb/__init__.py 88.37% <100.00%> (+0.10%) ⬆️
allensdk/brain_observatory/nwb/nwb_utils.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5200c74...d957572. Read the comment docs.

@pickles-bread-and-butter pickles-bread-and-butter changed the title Address unpin failures behavior ophys Add function to check for existing column name in list of column names Jun 15, 2020
@pickles-bread-and-butter pickles-bread-and-butter changed the base branch from rc/2.1.0 to address-behavior-ophys-NWB-failures June 16, 2020 18:04
@wbwakeman wbwakeman added behavior braintv relates to Insitute BrainTV program labels Jun 16, 2020
def get_column_name(table_cols: list,
possible_names: set) -> str:
"""
This function acts a identifier for which column name is present in the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reword this bit:

This function returns a column name, given a table with unknown
column names and a set of possible column names which are expected.
The table column name returned should be the only name contained in
the "expected" possible names.

Because stimulus presentation table column names (and contents)
differ between ecephys and visual behavior, this function determines
the proper column name to use in order to select the 'name' of a
presented stimulus (e.g. "gabors", "dot_motion", "natural_movie_1", etc.).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, changed the doc string to what you have provided.

Updated the Docstring to better capture the puprose of the function. This better aligns with the expectation of functional performance.
@pickles-bread-and-butter pickles-bread-and-butter merged commit 61845d8 into address-behavior-ophys-NWB-failures Jun 18, 2020
@pickles-bread-and-butter pickles-bread-and-butter deleted the address-unpin-failures-behavior-ophys branch June 18, 2020 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

behavior braintv relates to Insitute BrainTV program

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants