Align signals horizontally in plot_wfdb (if possible) #390
Conversation
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
|
Whoops, fixed a typo. |
Merged
cx1111
reviewed
Jun 14, 2022
wfdb/plot/plot.py
Outdated
| @@ -121,7 +121,7 @@ def plot_items( | |||
| signal=None, | |||
| ann_samp=None, | |||
| ann_sym=None, | |||
| fs=None, | |||
| frame_freq=None, | |||
There was a problem hiding this comment.
Demo 6 in demo.ipynb uses the old name.
wfdb/plot/plot.py
Outdated
| @@ -217,11 +216,11 @@ def plot_items( | |||
| sampling_freq : number or sequence, optional | |||
| The sampling frequency or frequencies of the signals. If this is a | |||
| list, it must have the same length as the number of channels. If | |||
| unspecified, defaults to `fs`. | |||
| unspecified, defaults to `frame_freq`. | |||
There was a problem hiding this comment.
Having these two parameters is rather confusing. Can you explain why both sampling_freq and frame_freq are both needed?
|
ACK, seems messier than I thought.
What if we forget about the frame_freq thing, forget about
sharex=True+units="samples", and instead just:
- use seconds by default in plot_wfdb
- use sharex=True in plot_wfdb *except* when units are set to samples
|
|
Yea, let's fix the
|
added 2 commits
June 24, 2022 14:56
This argument allows the caller to specify whether the horizontal axes should be synchronized across all channels (subplots) in a plot. In general, this behavior is desirable as it lets the viewer see the temporal relationship between channels. However, in the case where the record is multi-frequency *and* we are displaying sample numbers on the horizontal axis, matplotlib unfortunately does not appear to have a way to synchronize the axes automatically. Therefore, in the default "auto" case, enable X sharing as long as the time units for all channels are the same.
bemoody
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In
plot_wfdb, add an argumentsharexwhich controls whether X axes are shared between subplots.(This means that multiple channels will always appear time-aligned with each other, even if one channel starts or ends with a block of NaNs. It also means that if you use the pan/zoom buttons to navigate, the channels will stay aligned with each other.)
sharex=Trueis generally desirable behavior; however, if the time units are sample numbers and the record is multi-frequency, then it's not possible (AFAICT) with matplotlib to have the axes synchronized while displaying different units on different subplots. So we disable sharex by default in that case.Finally,
plot_wfdbwill usetime_units="seconds"by default, which I think is useful because it's consistent (across databases) and familiar.(original pull request description follows)
A few changes to improve plotting of multiple channels and multiple frequencies:Forplot_itemsand various internal functions, rename thefsargument toframe_freqso it is (perhaps) more clear what it means for multi-frequency data. (I'm still not completely satisfied and thinkplot_itemsis harder to use than it needs to be.)Allow specifyingtime_units="frames"as an alternative to "samples", "seconds", etc.The big change:plot_wfdbusessharex=True. This means that multiple channels will always appear time-aligned with each other, even if one channel starts or ends with a block of NaNs. It also means that if you use the pan/zoom buttons to navigate, the channels will stay aligned with each other.plot_wfdbwill usetime_units="seconds"by default, which I think is useful because it's consistent (across databases) and familiar.(fixes issue #373)