adding function for plotting specific interval lists side by side #1330
Conversation
There was a problem hiding this comment.
Thanks @gshvarts ! This looks good! There are two things I might add, and they can either happen in this PR if you want to take them on, or I can open an issue and migrate later.
- Testing - as you might have seen in other functions, the
return_figarg is used to our test suite can grab and inspect the content. This could be as simple as testing that all intervals were captured like this - Class Method - I'd like to open up the use of this function by invoking a method from the main class. Meaning something like ....
(IntervalList & my_restriction).plot_lists(start_time=0)This requires some familiarity with object-oriented programing, fetching the list names from self, but it would better align with the feedback I've seen from users requesting the ability to restrict a table before invoking a method
Lmk what you think. Like I said, you can either take a stab at the above or we can either review/merge as is
| "nwb_file_name": nwb_copy_file_name, | ||
| "interval_list_name": interval_list_name, | ||
| } | ||
| ).fetch1("valid_times") |
There was a problem hiding this comment.
This fetch loop will really slow down for greater numbers of intervals. I think you could optimize by running a single fetch of the name and times, and then running your convert function separately
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new helper function that plots specific interval lists side by side for visual validation and debugging.
- Added the function plot_specific_interval_lists() to generate a timeline plot using matplotlib.
- Introduced a new import for matplotlib as mpl and a helper function to convert interval endpoints to plot ranges.
Comments suppressed due to low confidence (2)
src/spyglass/common/common_interval.py:1157
- The comment and conversion here indicate a scaling by 60, which converts seconds to minutes, but the x-axis label states seconds. Consider updating the conversion factor, comment, and/or axis label to maintain consistency in units.
int_min = (interval[0] - start_time) / 60 # and convert to seconds
src/spyglass/common/common_interval.py:1129
- The docstring omits a description for the 'return_fig' parameter. Adding a brief explanation for 'return_fig' would improve clarity.
def plot_specific_interval_lists(nwb_copy_file_name, interval_list_names, start_time=0, return_fig=False):
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1330 +/- ##
==========================================
+ Coverage 67.21% 67.46% +0.24%
==========================================
Files 98 98
Lines 12272 12300 +28
==========================================
+ Hits 8249 8298 +49
+ Misses 4023 4002 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@CBroz1 I updated the existing One thing I couldn't quite figure out is that the times had some weird rounding issues after being plotted. I'm guessing that has something to do with the interval times getting converted into seconds for plotting and then maybe not being as precise to enough decimal points. I ended up just rounded the times arrays to the 4th decimal place before checking if they're equal to each other. I figured getting to that resolution for this plotting function isn't very important but let me know if it's something I should keep looking into. |
|
Added an error if there's more than one nwb_file_name in the IntervalList and a warning if there's >100 entries. Let me know if there's anything else I should add! |
Thanks @gshvarts - I'll test it out myself on Monday as a final check. The only other minor thing to correct is the lint check. This can be fixed by running... pip install black
black /path/to/this_file.pyIt may ask you to split up the longer strings like this: spyglass/src/spyglass/common/common_interval.py Lines 215 to 218 in fa9d97a to prevent any one line from going past the 80 char limit. If you use VSCode, you can look at mentions of |
Description
Adding the function
plot_specific_interval_lists()tocommon_interval.py.I added this function to my repo locally a while ago and use it all the time so figured it might be helpful for other people too. It's similar to the
IntervalList.plot_intervals()function except takes in a specific list ofinterval_list_namevalues to display and does some nice formatting. I've used it often as a helpful tool when double checking that my interval lists span the times that I expect them to and when developing new custom interval lists to add.Example usage:
Outputs:
Checklist:
CITATION.cffaltersnippet for release notes.CHANGELOG.mdwith PR number and description.