Update matplotlib to 3.8.4 - #4510

Now that pyodide/matplotlib-pyodide#52 has been merged, is this PR good to go?

The last time I was looking into this I thought there was a second real CI failure. But I just merged main, if the CI passes we can merge it.

Most of the test failures seem to be cannot import name 'maxdict' from 'matplotlib.cbook which I think was fixed by pyodide/matplotlib-pyodide#52. But there are also:

FAILED matplotlib/test_matplotlib.py::plot[chrome] - AttributeError: 'NoneType' object has no attribute 'get_element'
FAILED matplotlib/test_matplotlib.py::svg[chrome] - assert 15016 == 14998

which need to be investigated.

FAILED matplotlib/test_matplotlib.py::svg[chrome] - assert 15016 == 14998

The size of svg can change based on the implementation details, so I think we can just update the value or remove that assertion.

Yes, I'm not too concerned about that test either. I patched matplotlib-pyodide to fix the missing maxdict attribute, so this is what is left for the rest of the tests:

___________________ test_draw_image_affine_transform[chrome] ___________________
packages/matplotlib/test_matplotlib.py:305: in test_draw_image_affine_transform
    run(selenium, handle, ref)
packages/matplotlib/test_matplotlib.py:296: in run
    plt.show()
/lib/python3.12/site-packages/matplotlib/pyplot.py:527: in show
    ???
/lib/python3.12/site-packages/matplotlib_pyodide/html5_canvas_backend.py:453: in show
    ???
/lib/python3.12/site-packages/matplotlib_pyodide/browser_backend.py:183: in show
    ???
E   AttributeError: 'NoneType' object has no attribute 'get_element'

FAILED matplotlib/test_matplotlib.py::plot[chrome] - AttributeError: 'NoneType' object has no attribute 'get_element'

This error happens here, and it is set to None when initialized.

Maybe we can just add a if block to check the toolbar and skip the logic if it is None.

Let's see if these commits fix the tests. I have to rely on the CI because it's difficult to work with the browser tests locally – they fail consistently with timeouts for me (it's difficult to find versions even just slightly old off of Homebrew)

Okay, we got down to 8 failures out of 13 tests... now we have ValueError: 'bitmap' is not a valid value for output; supported values are 'path', 'agg', and 'macosx'. I'll keep looking into it.

I've hacked something together based on ipython/ipython#12889 and an attempt at a path-based MathTextParser because "bitmap" was deprecated since version 3.4, so it has been removed by 3.8 (or maybe in an earlier version). My Copilot's code completions seem to help a bit (the code looks wrong at so many places, until it's coerced to do what I want it to do :P). Is there a reliable way to go through the code -> test -> debug -> code loop, and faster? For such a change, testing in CI is going to be quite slow.

Thanks for the pointer – redirected the backend, and both the HTML5 and WASM backends now use the WASM backend underneath. I've confirmed the changes with https://github.com/pyodide/pyodide/actions/runs/12354843167/artifacts/2326627934 as well. Let's move ahead with this PR in a moment!

pyodide/matplotlib-pyodide#60 has been completed, so I've updated matplotlib-pyodide to version 0.2.3 in b20bc94. I would have liked to bump right up to Matplotlib 3.9.2, but I am not too keen on it – we could have a 0.27.1 release for that and a bunch of other package updates, perhaps...

You definitely need contourpy as a dependency:
https://github.com/matplotlib/matplotlib/blob/8ac84ecb06ffa735f3da578f25aee1ee6f213cac/pyproject.toml#L33-L43

You definitely need contourpy as a dependency: matplotlib/matplotlib@8ac84ec/pyproject.toml#L33-L43

Yeah, noticed it from a local build just now, I guess that's why I added it back then. Added again in 53786f3.

We should be good to merge this once CI passes. @hoodmane, it is your PR so I can't seek a review from you from the interface directly.

Yeah I'm not allowed to approve my own PR. And apparently there is no way to transfer ownership of the PR to you without opening a separate PR. But if we agree about the changes we can merge it github approval or no.

Looks good to me @agriyakhetarpal. Please feel free to merge when packages tests pass.

Argh, it is LightGBM yet again. I haven't been able to figure out what other package messes with it when building (which is what happens, according to Gyeongjae), because it never causes problems for me locally. Should we isolate it into its own CI job like we have for OpenCV?

because it never causes problems for me locally. Should we isolate it into its own CI job like we have for OpenCV?

After 0.27.0 release, I'll proceed to unvendoring packages, so I guess we'll be able to split package build jobs.

Thanks @agriyakhetarpal and @ryanking13!

I would have liked to bump right up to Matplotlib 3.9.2, but I am not too keen on it – we could have a 0.27.1 release for that and a bunch of other package updates, perhaps...

There are changes in Matplotlib 3.9 and 3.10 affecting backends which might need patching here. I suggest that 3.8.4 is recent enough for the moment and we hold off doing any more updates until I've had a chance to look into the modified WebAgg backend idea, if that is OK.

I would have liked to bump right up to Matplotlib 3.9.2, but I am not too keen on it – we could have a 0.27.1 release for that and a bunch of other package updates, perhaps...

There are changes in Matplotlib 3.9 and 3.10 affecting backends which might need patching here. I suggest that 3.8.4 is recent enough for the moment and we hold off doing any more updates until I've had a chance to look into the modified WebAgg backend idea, if that is OK.

Makes sense, thanks, @ianthomas23, for noting and thanks again for taking out the time to collaborate with us on this (and for helping review this PR, of course!).