-
Notifications
You must be signed in to change notification settings - Fork 22
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
[WIP] Trialized time series faceting #232
Merged
bendichter
merged 19 commits into
NeurodataWithoutBorders:master
from
catalystneuro:trialized_time_series_faceting
Dec 6, 2022
Merged
Changes from 13 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
21bb54e
first draft
h-mayorquin 97fecc5
added first plot
h-mayorquin 89097cb
add option when not available
h-mayorquin fd57c1c
added timeseries control
h-mayorquin 860624b
hide filtering selection if not used
h-mayorquin 3493f2f
format and keeping state with another variable
h-mayorquin 09a556a
Merge branch 'master' into trialized_time_series_faceting
h-mayorquin 5269e42
added mechanism for not doubling timestamps
h-mayorquin eb1499a
added series scaling
h-mayorquin 68d6192
make it work with nwb2widget main function
h-mayorquin 61db805
Merge branch 'master' into trialized_time_series_faceting
h-mayorquin 761583b
add toggle for x-locking behavior
h-mayorquin 375c60b
Merge branch 'trialized_time_series_faceting' of https://github.com/c…
h-mayorquin 42ca8e7
use time series units
h-mayorquin 4b26e16
displayed selection filter options filtered
h-mayorquin 0b4235c
separate conrtroller for visualization widget
h-mayorquin d50e883
ordered faceting columns and rows
h-mayorquin 4aed1b7
added router for ROI and electrical series
h-mayorquin 2bb7ba3
fix dropvalues
h-mayorquin File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a problem here when we make the
neurodata_vis_spec[RoiResponseSeries]
anOrderedDict
to include theTrializedTimeSeries
as an option (see the diff in theview.py
module).Because
show_df_over_f
is a callable the functionnwb2widget
expects this output to be a visualization (not an ordered dict) and a naive extension of the visualization specification generates an error. The solution I implemented was to roll back theshow_df_over_f
to return the visualization for the wholeContainer/DataInterface
(see the else in the diff representing the old code) and then the visualization can be properly display as a root of thelazy_tab
for that module. That is how the widget is working right now.The other option was to modify
nwb2widget
to handle the case when callables returnOrderedDict
but that seemed like a larger modification given thatnwb2widget
is such a central function and this might have non-tested side effects in other visualizations. I opted for the simple solution here as that only touches the visualizations concerning the RoiExtractors and DfOverF objects.