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

Compatibility with latest MPL 3.8 (Dev) #433

Closed
wants to merge 6 commits into from

Conversation

cvanelteren
Copy link

Hi,

I really enjoy the interface of proplot! Unfotunately, proplot is not compatible with the latest mpl version. I have edited the source to make it compatible with mpl 3.8 (dev).

Issues were raised in #390.

I don't see any unittests in project other than test_journal.py. Let me know if this PR works for you.
Sincerely,
C

@cvanelteren cvanelteren changed the title colormap fix mpl v3.8 Compatibility with latest MPL 3.8 (Dev) Jul 29, 2023
@lukelbd
Copy link
Collaborator

lukelbd commented Jul 29, 2023

Hi, thanks for the PR!

Would you mind undoing the changes from single to double quotes? It's a little hard to read your PR right now. Single quotes are the standard I've settled on (I mostly follow black --skip-string-normalization). Also please double check that you're using the latest master branch if you can (I'm seeing conflicts?).

Unfortunately I've already started to work on matplotlib 3.7 compatibility, but haven't started a PR yet. Maybe before you commit much more work to this, could you wait a few days? I have not yet started matplotlib 3.8 testing so I'm sure your PR could help with 3.7-3.8 differences, but some changes could be redundant / there could be initial conflicts. I'd also be happy to add your changes to that PR if that's easier (you'll still be added to the contributors list).

Also please monitor #413 re: tests. I'm planning to add a broad testing suite before the next release. It will include test image comparisons / run tests against an array of different matplotlib versions.

@cvanelteren
Copy link
Author

Sounds good!I think I ran black on it indeed before pushing, the single quotes were not intended. The pull was from the latest release (master). I'm not behind my machine right now but will check it when I have the chance.

The biggest issue was figuring out how the cmaps are registered under proplot. Mpl changed it from a dict to a class somewhere above 3.5.

The other issue was concerning the tick formatter. Somehow the function _not_none returned None on minorticklocator.

The rest is pretty much the same. Unittests would be amazing for the future. Happy to hear they are on the way. Keep up the good work.

Cheers! C

@cvanelteren
Copy link
Author

I reset the commit @lukelbd. I did a local merge of the tests branch but it generates some errors using pytest (missing decorator definitions). I used the environment.yml listed in PR 413. Let me know if I can assist you in the mpl 3.7 or 3.8 merge.
Best wishes,
C

@riley-brady
Copy link

Nudging this, as I am having similar issues.

@cvanelteren it looks like the breaks are due to matplotlib imports in the docs for similar version issues

AttributeError: module 'matplotlib.cm' has no attribute '_gen_cmap_registry'

@cvanelteren
Copy link
Author

@riley-brady I'll correct the doc test. The patch proposed is a bit of a monkey patch albeit functional. I believe @lukelbd is providing more profound changes. Would be willing to help to get this going, but this would require some coordination I think.

proplot/colors.py Outdated Show resolved Hide resolved
Co-authored-by: Scott Staniewicz <scott.stanie@gmail.com>
@zxdawn
Copy link

zxdawn commented Nov 27, 2023

I have tested the branch with fig, axs = pplt.subplots(proj='cyl') and got this error:

lib/python3.11/site-packages/matplotlib/ticker.py", line 2912, in __call__
    if self.axis.axis_name == 'y':
       ^^^^^^^^^^^^^^^^^^^
AttributeError: '_LonAxis' object has no attribute 'axis_name'

@cvanelteren
Copy link
Author

This branch is not actively maintained by me any more. Luke is busy on updating proplot to be compatible with the latest mpl.

@cvanelteren
Copy link
Author

cvanelteren commented Mar 2, 2024

I tracking this again on https://github.com/cvanelteren/proplot/tree/mpl3.8.3, and intend to push when I am more certain on the compatibility of the code base. As noted by @lukelbd there are currently not tests to check the internal validity. Since the gap between proplot and mpl is widening it would sadden me if we loose the work put into this package.

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

Successfully merging this pull request may close these issues.

None yet

5 participants