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

[CR] matplotlib modernization #380

Merged
merged 19 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
350 changes: 243 additions & 107 deletions mir_eval/display.py

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion mir_eval/sonify.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def pitch_contour(
time indices for each frequency measurement, in seconds
frequencies : np.ndarray
frequency measurements, in Hz.
Non-positive measurements will be interpreted as un-voiced samples.
Non-positive measurements or NaNs will be interpreted as un-voiced samples.
fs : int
desired sampling rate of the output signal
amplitudes : np.ndarray
Expand Down Expand Up @@ -252,6 +252,8 @@ def pitch_contour(
# Squash the negative frequencies.
# wave(0) = 0, so clipping here will un-voice the corresponding instants
frequencies = np.maximum(frequencies, 0.0)
# Convert nans to zeros to unvoice
frequencies = np.nan_to_num(frequencies, copy=False)

# Build a frequency interpolator
f_interp = interp1d(
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tool:pytest]
addopts = --cov-report term-missing --cov mir_eval --cov-report=xml
addopts = --cov-report term-missing --cov mir_eval --cov-report=xml --mpl --mpl-baseline-path=baseline_images/test_display

[pydocstyle]
# convention = numpy
Expand Down
Binary file removed tests/baseline_images/test_display/events.png
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file removed tests/baseline_images/test_display/piano_roll.png
Binary file not shown.
Binary file not shown.
Binary file removed tests/baseline_images/test_display/pitch_hz.png
Binary file not shown.
Binary file removed tests/baseline_images/test_display/pitch_midi.png
Binary file not shown.
Binary file removed tests/baseline_images/test_display/pitch_midi_hz.png
Binary file not shown.
Binary file removed tests/baseline_images/test_display/segment.png
Binary file not shown.
Binary file removed tests/baseline_images/test_display/segment_text.png
Binary file not shown.
Binary file removed tests/baseline_images/test_display/separation.png
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this is just due to plot ranges, but there's now a little blip on "z" at time zero. Also a nit but it's a bit odd that the upper plot has some whitespace at the top.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the whitespace is due to matplotlib margin handling, and it's on all edges. This is configurable at the rcParams level or by calling ax.margins(), but it's not something that artist constructors should be styling directly.

The blip on "z" is correct if you look at the source annotations: https://github.com/craffel/mir_eval/blob/main/tests/data/hierarchy/ref00.lab and https://github.com/craffel/mir_eval/blob/main/tests/data/hierarchy/ref01.lab - they were always in the plots, but the old styling made them difficult to see due to axis spines.

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment on the whitespace on the top here, so is a common feature

Diff not rendered.
Loading
Loading