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

Documentation builds fail - Mapgenerator breaks with matplotlib >= 3.6 #2800

Closed
zklaus opened this issue Sep 19, 2022 · 19 comments · Fixed by #2830 or #3068
Closed

Documentation builds fail - Mapgenerator breaks with matplotlib >= 3.6 #2800

zklaus opened this issue Sep 19, 2022 · 19 comments · Fixed by #2830 or #3068

Comments

@zklaus
Copy link
Contributor

zklaus commented Sep 19, 2022

The mapgenerator package is incompatible with matplotlib >= 3.6 because it uses the function matplotlib.cbook.deprecated, which has been deprecated in matplotlib 3.4 and has finally been removed in matplotlib 3.6; this breaks our tests.

I will patch the conda-forge package of mapgenerator to to introduce an upper limit, but this is a temporary fix since it blocks us from updating matplotlib.

For a permanent fix it would be good to fix mapgenerator itself.

@sloosvel, since the package originates from BSC, do you think you could drive this forward?

Perhaps @schlunma, you could help because, as I understand it, the problem is that it uses some API from matplotlib that was used to mark deprecations and that has been removed or made private to matplotlib as the general Python deprecation handling matured, and you have some experience with deprecations?

@valeriupredoi
Copy link
Contributor

eagle eyed Klaus 🦅 I was just about to open the same thing! Here is the test fail - currently tests pass since the environment has automagically reverted to matplotlib 3.5.3 (I have seen a bunch of unrelated failures in the 3.6.0 rc so prob the Matplotlib guys are looking into those atm); shall we pin Matplotlib for now?

@valeriupredoi
Copy link
Contributor

I suggest we find (even an interim) solution to this rather sooner than later, please - our tests are starting to drop like proverbial flies, see the conda lock test now - I can push a temp pin right away if that's OK with you, Manu is on holidays so maybe Saskia can give us some help with the BSC package, but in the meantime I suggest we pin the plotster

@sloosvel
Copy link
Contributor

I have reported that already. I will let you know when it's solved.

@valeriupredoi
Copy link
Contributor

cheers, Saskia 🍺

@schlunma
Copy link
Contributor

An example of a deprecation in ESMValCore can be found here:

https://github.com/ESMValGroup/ESMValCore/blob/d17a1525896275d3919f7b290f0514ca3127bee9/esmvalcore/iris_helpers.py#L109-L138

It basically uses the Sphinx directive .. deprecated:: and then raises a warning with warnings.warn. This is also what other libraries (e.g., iris) are doing.

An alternative could be the decorator @deprecated from deprecated.sphinx that claims to automatically add the directive and emits a warning. However, this needs an extra dependency.

@bouweandela
Copy link
Member

Would it make sense to pin matplotlib for now as proposed by @valeriupredoi? Our documentation build appears to be failing because of this (see #2824).

@valeriupredoi valeriupredoi changed the title Mapgenerator breaks with matplotlib >= 3.6 Documentation builds fail - Mapgenerator breaks with matplotlib >= 3.6 Oct 4, 2022
@valeriupredoi
Copy link
Contributor

if there are no moves on this by Friday, I will open a PR with a matplotlib pin - but what's the situation with Mapgenerator at good old BSC, @sloosvel

@sloosvel
Copy link
Contributor

sloosvel commented Oct 4, 2022

I don't think it will be ready for this release. You can follow or ask in this issue: https://earth.bsc.es/gitlab/es/mapgenerator/-/issues/80

@valeriupredoi
Copy link
Contributor

thanks @sloosvel - the level of appetite for solving the issue seems to be somewhat minimal gauging from the discussion on that thread, I'll pin Matplotlib soon then 👍

@valeriupredoi
Copy link
Contributor

good catch @bouweandela - why did silly GH API close this, I didn't say Closes XXX

@bouweandela
Copy link
Member

GitHub still needs to learn what 'do not close' means 😄

@valeriupredoi
Copy link
Contributor

at least GH is not a grammar naz* 😆

@zklaus
Copy link
Contributor Author

zklaus commented Feb 24, 2023

@sloosvel, any progress on this upstream? I am keen to move forward with matplotlib. If I recall correctly, the fix is not terribly hard to do. Let me know if you need help.

@sloosvel
Copy link
Contributor

Let me ask again

@sloosvel
Copy link
Contributor

Ok version 1.0.6 is now available on pypi, the decorator that was causing issues is now removed.

@zklaus
Copy link
Contributor Author

zklaus commented Feb 28, 2023

That's great! Would you or one of your colleagues like to become co-maintainer on the conda-forge feedstock? I'm happy to continue, so no pressure; just the bus factor.

@zklaus
Copy link
Contributor Author

zklaus commented Mar 1, 2023

Mapgenerator is now released in a fixed version also on conda-forge, so we should be able to drop our pin on matplotlib.

@valeriupredoi
Copy link
Contributor

muzik to me ears, let me open a PR with the pin drop then - cheers for the good work, guys 🍻

@valeriupredoi valeriupredoi mentioned this issue Mar 1, 2023
5 tasks
@sloosvel
Copy link
Contributor

sloosvel commented Mar 1, 2023

That's great! Would you or one of your colleagues like to become co-maintainer on the conda-forge feedstock? I'm happy to continue, so no pressure; just the bus factor.

FYI @fbeninca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants