Skip to content
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

'change_time_no_display_delay' key in trial table breaks session.get_rolling_performance_df() method #2490

Closed
egmcbride opened this issue Jul 7, 2022 · 5 comments · Fixed by #2701
Assignees
Labels

Comments

@egmcbride
Copy link

Describe the bug
The session.get_rolling_performance_df() method does not work in the vbn_2022_dev branch

To Reproduce

from pynwb import NWBHDF5IO
from allensdk.brain_observatory.ecephys.behavior_ecephys_session import (
BehaviorEcephysSession)

path=r"\allen\programs\mindscope\workgroups\np-behavior\vbn_data_release\vbn_s3_cache\visual-behavior-neuropixels-0.1.0\ecephys_sessions\ecephys_session_1044594870.nwb"

with NWBHDF5IO(path, 'r', load_namespaces=True) as nwb_io:
session = BehaviorEcephysSession.from_nwb(nwbfile=nwb_io.read())

session.get_rolling_performance_df()

Expected behavior
produce a table with the rolling performance metrics.
I believe the new key 'change_time_no_display_delay' breaks the code that is looking for just 'change_time'

Actual Behavior

AttributeError Traceback (most recent call last)
~\AppData\Local\Temp\ipykernel_21176\3325833841.py in <cell line: 2>()
1 # session.trials['change_time'] = session.trials['change_time_no_display_delay']
----> 2 session.get_rolling_performance_df()

c:\users\ethan.mcbride\code\allensdk_vbn_2022\allensdk\brain_observatory\behavior\behavior_session.py in get_rolling_performance_df(self)
665
666 """
--> 667 return construct_rolling_performance_df(
668 self.trials,
669 self.task_parameters['response_window_sec'][0],

c:\users\ethan.mcbride\code\allensdk_vbn_2022\allensdk\brain_observatory\behavior\trials_processing.py in construct_rolling_performance_df(trials, response_window_start, session_type)
252
253 """
--> 254 reward_rate = calculate_reward_rate_fix_nans(
255 trials,
256 response_window_start)

c:\users\ethan.mcbride\code\allensdk_vbn_2022\allensdk\brain_observatory\behavior\trials_processing.py in calculate_reward_rate_fix_nans(trials, response_window_start)
194
195 """
--> 196 response_latency_list = calculate_response_latency_list(
197 trials,
198 response_window_start)

c:\users\ethan.mcbride\code\allensdk_vbn_2022\allensdk\brain_observatory\behavior\trials_processing.py in calculate_response_latency_list(trials, response_window_start)
165 for _, t in trials.iterrows():
166 valid_response_licks =
--> 167 [x for x in t.lick_times
168 if x - t.change_time > response_window_start]
169 response_latency = (

c:\users\ethan.mcbride\code\allensdk_vbn_2022\allensdk\brain_observatory\behavior\trials_processing.py in (.0)
166 valid_response_licks =
167 [x for x in t.lick_times
--> 168 if x - t.change_time > response_window_start]
169 response_latency = (
170 float('inf')

C:\Anaconda3\envs\allensdk_38\lib\site-packages\pandas\core\generic.py in getattr(self, name)
5573 ):
5574 return self[name]
-> 5575 return object.getattribute(self, name)
5576
5577 def setattr(self, name: str, value) -> None:

AttributeError: 'Series' object has no attribute 'change_time'

Environment (please complete the following information):

  • OS & version: Windows 10
  • Python version 3.8
  • AllenSDK version: vbn_2022_dev - latest version

Additional context
adding the following line fixes the problem:
session.trials['change_time'] = session.trials['change_time_no_display_delay']

@egmcbride egmcbride added the bug label Jul 7, 2022
@aamster aamster self-assigned this Jul 8, 2022
@aamster
Copy link
Contributor

aamster commented Jul 8, 2022

@egmcbride @corbennett can I just rename this column to "change_time" so that it is consistent with what the current code expects or do you want to keep it as is ("change_time_no_display_delay")?

@corbennett
Copy link
Contributor

@aamster we would prefer to modify the sdk so that this code works with either the old column name ('change_time') or the new one ('change_time_no_display_delay'). The old label is pretty misleading for this dataset, so we'd like to avoid reverting if we can. But if this change seems too complicated/difficult before the release, we can revisit this option.

@aamster
Copy link
Contributor

aamster commented Jul 12, 2022

@egmcbride @corbennett this should be fixed on rc/2.13.5

@aamster aamster closed this as completed Sep 15, 2022
@corbennett
Copy link
Contributor

@aamster It seems that this is still broken for allensdk version 2.15.1. Is that expected?

@aamster
Copy link
Contributor

aamster commented Jul 19, 2023

@corbennett I am able to reproduce it also. I will fix it on the main branch and it will be deployed to pypi on the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants