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

adding censoring to event triggered response #39

Merged
merged 6 commits into from
Sep 4, 2024
Merged

adding censoring to event triggered response #39

merged 6 commits into from
Sep 4, 2024

Conversation

alexpiet
Copy link
Collaborator

  • When we create the event triggered response, if multiple events are close together, we may want to "censor" the event triggered response by ignoring timepoints that happen after the subsequent second event, or before a proceeding event.
  • I added an option to alignment.event_triggered_response(censor=True) that lets the user optional censor the data. Note this only works in the "tidy" format at the moment.

@alexpiet
Copy link
Collaborator Author

@hagikent @rachelstephlee @ZhixiaoSu @hanhou This is ready for review

Copy link
Collaborator

@rachelstephlee rachelstephlee left a comment

Choose a reason for hiding this comment

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

you should add a testing function for this to test that this works correctly. this should be in tests/test_aind_dynamic_foraging_data_utils.py (which can be split into two for nwb vs alignment).

@alexpiet
Copy link
Collaborator Author

alexpiet commented Sep 4, 2024

@rachelstephlee I've added a unit test

Copy link
Collaborator

@rachelstephlee rachelstephlee left a comment

Choose a reason for hiding this comment

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

looks great thank you !

@alexpiet alexpiet merged commit 5116f3c into main Sep 4, 2024
3 checks passed
@alexpiet alexpiet deleted the add_censor branch November 13, 2024 03:37
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.

2 participants