Support matplotlib 3.8

[bmcfee]

Matplotlib 3.8 removes some (private) functionality that we've been relying on in the display module:

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L150

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L273

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L464

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L555

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L623

https://github.com/craffel/mir_eval/blob/9be1c5bf9b02b842b22f5b248189fbf88a7e30ac/mir_eval/display.py#L782-L786

We probably should never have been doing this — my bad! :sweat_smile: — but now we have to remove / rewrite this functionality to maintain compatibility.

This is going to take a bit of work, and I probably can't commit to doing this in the short term. But I'd be happy to help out if anyone wants to take this on.

[craffel]

Thanks for noting this. In the short-term, should we at least enforce a < 3.8 requirement for matplotlib in setup.py?

[bmcfee]

Thanks for noting this. In the short-term, should we at least enforce a < 3.8 requirement for matplotlib in setup.py?

Yeah, I think that would make sense - both for the display and testing options. I'm not sure if it's worth cutting a post-release for, or pushing ahead with a proper next version?

[craffel]

I know this is the wrong answer but I think few enough people are using both mir_eval.display and matplotlib >= 3.8.0 that we can probably get away with just leaving it at HEAD until the next proper release.

[bmcfee]

That's almost certainly true. Quick github search has 51 code hits: https://github.com/search?q=mir_eval.display&type=code and most of them are either our own (for appropriately defined "ours") or one-off code from a few years back.

That said, it would be good to set a milestone for the next release so we have a sense of how long it will take.

[craffel]

I don't have a sense of what we'd include in a next milestone... maybe once #355 is in?

[bmcfee]

Jotting down some thoughts here...

The basic problem is that we want something that works essentially like ax.plot - in that subsequent calls automatically increment the style cycler, so that we can draw multiple annotations overtop each other and have it work the way one would expect. To make this happen, we previously hooked into the axes object's line style cycler https://github.com/craffel/mir_eval/blob/7997fdf3972f992209eaf144f37c18926fa3c960/mir_eval/display.py#L553 or patch cycler https://github.com/craffel/mir_eval/blob/7997fdf3972f992209eaf144f37c18926fa3c960/mir_eval/display.py#L148

This is no longer allowed in mpl 3.8.

We bumped into this issue in librosa's waveshow function a little while ago, and circumvented it by calling ax.plot first, then hijacking the style from the resulting object to propagate to other linked objects (fill_between for envelope display). This approach won't work in mir_eval because the underlying artists are not usually line plots.

After a bit of hacking and prodding ye olde chatbot, I think the most reliable solution here might be for us to add a custom cycler object to the axes object in question, and only interface with that. Something like:

ax = plt.gca()

if not hasattr(ax, "mir_eval_cycler"):
    ax.mir_eval_cycler = ... # instantiate a custom cycler object from matplotlib.rcParams

style = next(ax.mir_eval_cycler)  # or something

This, I think, would work forward and backward across matplotlib versions, and should keep us entirely independent of other artists on the axes. That last point is a double-edged sword, as ax.plot and mir_eval.display.pitch_contour (or whatever) could potentially be independently cycling through the same styles and end up indistinguishable.

We may therefore want to provide some API for a user-provided cycler object to use or initialize the axes custom cycler with.

There may be downsides to this approach that I'm not yet seeing. I'll prototype it some time and then see if some mpl devs can comment on how bad of an idea it is.

[bmcfee]

Gave this a bit more of a think, and decided we should avoid stashing custom stylers in the axes object.

Instead, I think we can do a slightly less direct version of what we had already been doing, where rather than poll the appropriate style cycler directly (verboten), we create a temporary line or patch object that does this for us, yank its style for the new artists we're creating, and then purge the object from the axes.

This also is a little clumsy, but it's at least adhering to the API in a way that shouldn't break any time soon.

Aside: since we're bumping the minimum matplotlib version, we can significantly simplify the pitch contour (and probably multipitch) viz by using nans for unvoiced regions. This lets us use a single plot object rather than plotting each segment separately.