Fix ImportError

[carlthome]

As of https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.9.0.html#miscellaneous-removals the tests fail because matplotlib.collections.BrokenBarHCollection has been removed now. This PR attempts to fix support for newer matplotlib versions.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.73%. Comparing base (485a425) to head (45a55b4).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #385      +/-   ##
==========================================
+ Coverage   95.67%   95.73%   +0.06%     
==========================================
  Files          19       19              
  Lines        2936     2934       -2     
==========================================
  Hits         2809     2809              
+ Misses        127      125       -2     

Flag	Coverage Δ
unittests	95.73% <100.00%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bmcfee]

🤦 can't believe I fixed the use of this in #380 and didn't bother to remove the import! Thanks.

Tests are failing now because of mpl2.0 deprecations. How about we roll in some fixes to that in this PR?

[bmcfee]

Looks like remaining failures are due to matplotlib changes... I'll see if I can sort out what's happening here, but it's likely a freetype thing.

[bmcfee]

Ah it's actually much simpler - the problem is the change in default legend location. I'll suggest some fixes that should get this passing again.

[bmcfee]

The following diff will fix the legend issues:

diff --git a/tests/test_display.py b/tests/test_display.py
index d2c254b..c87cdcd 100644
--- a/tests/test_display.py
+++ b/tests/test_display.py
@@ -151,7 +151,7 @@ def test_display_labeled_intervals_compare_noextend():
         est_int, est_labels, extend_labels=False, alpha=0.5, label="Estimate"
     )
 
-    plt.legend()
+    plt.legend(loc='upper right')
     return plt.gcf()
 
 
@@ -178,7 +178,7 @@ def test_display_labeled_intervals_compare_common():
         est_int, est_labels, label_set=label_set, alpha=0.5, label="Estimate"
     )
 
-    plt.legend()
+    plt.legend(loc='upper right')
     return plt.gcf()
 
 
@@ -344,7 +344,7 @@ def test_display_piano_roll():
         est_t, est_p, label="Estimate", alpha=0.5, facecolor="r"
     )
 
-    plt.legend()
+    plt.legend(loc='upper right')
     return plt.gcf()
 
 
@@ -367,7 +367,7 @@ def test_display_piano_roll_midi():
         est_t, midi=est_midi, label="Estimate", alpha=0.5, facecolor="r"
     )
 
-    plt.legend()
+    plt.legend(loc='upper right')
     return plt.gcf()

There's still one remaining failure with segment_text where the inset annotations are not properly clipping to the segment boxes. This appears to be a regression in matplotlib 3.9, since it works properly in 3.8.1. I'll raise an issue over in matplotlib, but for now I propose that we mark this test as skip if the matplotlib version matches 3.9.

I think we can add an xfail here on top of the existing one that bypasses these tests for mpl < 3.8, or we can just tighten the existing xfail logic to only run on 3.8 for now.

https://github.com/craffel/mir_eval/blob/485a42551f449517567313f9871f58e9c99fbd8a/tests/test_display.py#L25

becomes

# Clamp this down when MPL fixes itself
OLD_MPL = (MPL_VERSION < version.parse("3.8.0")) or (MPL_VERSION >= version.parse("3.9.0"))

As it turns out, OLD_MPL xfail is only used for this one offending test anyway, so this should be fine. We might want to rename it from OLD to BAD or something for a bit more clarity, but I think these fixes should get this PR passing.

[bmcfee]

Annotation clipping issue pushed upstream to matplotlib/matplotlib#28717

[bmcfee]

Update, got a fix from the mpl devs for the annotation clipping bug:

diff --git a/mir_eval/display.py b/mir_eval/display.py
index 3a28f07..0dee99c 100644
--- a/mir_eval/display.py
+++ b/mir_eval/display.py
@@ -183,18 +183,15 @@ def segments(
         seg_map[lab].pop("label", None)
 
         if text:
-            bbox = Bbox.from_extents(ival[0], base, ival[1], height)
-            tbbox = TransformedBbox(bbox, transform)
             ann = ax.annotate(
                 lab,
                 xy=(ival[0], height),
                 xycoords=transform,
                 xytext=(8, -10),
                 textcoords="offset points",
-                clip_path=rect,
-                clip_box=tbbox,
                 **text_kw
             )
+            ann.set_clip_path(rect)
 
     return ax

[carlthome]

Great! This PR seems good to go in now, and then a subsequent 0.7.1 bump would be great to get out on PyPI for people. 👏

[bmcfee]

Awesome. Thanks @carlthome !

[drscotthawley]

Just hit this error on my first time ever attempting to use mir_eval. I installed via the instructions on the docs page:

pip install mir_eval

When might we expect an update of the PyPI package?

[craffel]

When might we expect an update of the PyPI package?

In the next few weeks.