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

ticket/PSB155: Add is_sham_change column. #2715

Merged
merged 2 commits into from
Aug 21, 2023
Merged

Conversation

morriscb
Copy link
Contributor

@morriscb morriscb commented Aug 9, 2023

Add is_sham_change to stimulus presentations.

Add trials as an optional input to allow for calculating new columns: active, is_sham_change.
Move trials_id calculation to stimulus presentations init to better handle all presentations table creation modes.
Add int typing enforcement to trials table.
Add active, and is_sham_change to stimulus processing. Simplify trials_id calculation.
Add unittests for active and is_sham_change column calculation and update other unittests.

Note: Will update linting failures before merge to avoid chaff from non-change files. Updating on #2712

@morriscb morriscb marked this pull request as ready for review August 15, 2023 19:07
Copy link
Contributor

@mikejhuang mikejhuang left a comment

Choose a reason for hiding this comment

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

Why did Corbett set a condition, (stimulus_block==5) & (is_sham_change) that is commented as the passive block and you only needed to use the condition ~stim_df.active? Corbett used stim_df.active to subset the active rows but didn't use the inverse to subset the passive rows.

#2643 (comment)

@morriscb
Copy link
Contributor Author

Why did Corbett set a condition, (stimulus_block==5) & (is_sham_change) that is commented as the passive block and you only needed to use the condition ~stim_df.active? Corbett used stim_df.active to subset the active rows but didn't use the inverse to subset the passive rows.

#2643 (comment)

Corbett is assuming that block 5 is a replay block, that is the same stimulus is replayed to the mouse. Both his and my code copies information from the trials table and related columns (is_sham_change being one) from the active into this reply block. The code I wrote is more general in case in the future there are 1) more than one active block 2) more than one replay block. In short it's doing the same thing but doesn't relying on assuming the block numbers.

Move trials_id and simplify calculation.
Add active calculation.
Add loading trails to presentations table creation code.
Add unittests for new stimulus calculations.
Change test for trials_id to account for gaps in trials.
@morriscb morriscb merged commit 420d6e2 into rc/2.16.0 Aug 21, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants