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

feat(block_analysis): add table for BlockForagingBout #432

Conversation

ttngu207
Copy link
Contributor

@ttngu207 ttngu207 commented Oct 4, 2024

@jkbhagatio , I also rearrange get_threshold_associated_pellets slightly to add more checks for empty dataframes.

bout_start: datetime(6)
---
bout_end: datetime(6)
bout_duration: float # (seconds)
Copy link
Member

@jkbhagatio jkbhagatio Oct 4, 2024

Choose a reason for hiding this comment

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

I'm happy to remove duration here and in the foraging_bouts function as it is trivial to recompute from the start and end times

Copy link
Member

Choose a reason for hiding this comment

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

So I guess the docstring and comments in that function, and any code computing the durations should be updated as welll?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's because I saw the bout_durations being computed in the function but then not used at all. I thought it was meant to be added to the result dataframe.
I think it's convenient to add in the table, but we can omit and compute on the fly if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I now see that you mentioned this in #427, sorry I missed this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@jkbhagatio
Copy link
Member

Cool, looks good, thanks!

@jkbhagatio jkbhagatio merged commit 5dacb45 into SainsburyWellcomeCentre:datajoint_pipeline Oct 5, 2024
1 check 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.

2 participants