AllenInstitute / AllenSDK

code for reading and processing Allen Institute for Brain Science data
https://allensdk.readthedocs.io/en/latest/
Other
335 stars 149 forks source link

`stimulus_presentations` table currently lists the stimulus file name as `image_set` #2569

Closed DowntonCrabby closed 1 year ago

DowntonCrabby commented 1 year ago

Describe the bug Currently the stimulus_presentations attribute, (for behavior_session or ophys_session dataset objects) currently lists the stimulus file name in the image_set column, when the stimulus file name doesn't actually directly provide information on the image_set.

This may also be the case for VBN as well, but @corbennett may want to confirm, and confirm if they would also like it changed.

stimulus_file_name_image_set

To Reproduce

import allensdk
from allensdk.brain_observatory.behavior.behavior_project_cache import VisualBehaviorOphysProjectCache

local_data_storage_directory = 
cache = VisualBehaviorOphysProjectCache.from_s3_cache(cache_dir=local_data_storage_directory)

ophys_session = cache.get_behavior_ophys_experiment(ophys_experiment_id=951980471)
ophys_session.stimulus_presentations

Expected behavior We should remove the stimulus file name from this table completely and instead include it in the session metadata attribute with the key "stimulus_file_name".

Instead of providing the stimulus file name under image set, I think we should either: (@matchings should weigh in on which is a better option)

Environment (please complete the following information):

Additional context Just providing the letter of the image set would require more parsing

morriscb commented 1 year ago

Hey Clark, could you load this same experiment with the from_lims cache option on the rc/2.16.1 development branch and see if the table is more to your liking? The image set column is dropped and more columns are included.

morriscb commented 1 year ago

Hey @DowntonCrabby, were you able to look at the current stimulus table on rc/2.16.1 yet and see if the columns there are satisfactory?

morriscb commented 1 year ago

Hey @DowntonCrabby, since I have a session loaded up with the current release candidate, here are the columns that are present in the data. As you can see, when pulling the data from LIMS on SDK rc we are working on, there is no longer a image_set column, with additional columns made available.

Screen Shot 2022-10-12 at 2 44 54 PM
morriscb commented 1 year ago

Ticket resovled. @DowntonCrabby Will open new issue regarding adding VBN similar columns to VBO (e.g. image_set, map NaN to grattings).