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.

Sign up for GitHub

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

Jump to bottom

ENH: future proof against matplotlib's evolving API #44

Merged
merged 1 commit into from
Sep 17, 2022
Merged

ENH: future proof against matplotlib's evolving API #44

merged 1 commit into from
Sep 17, 2022

Conversation

neutrinoceros
Copy link
Collaborator

fix #43

@neutrinoceros neutrinoceros force-pushed the mpl_future_proofing branch 2 times, most recently from f7dd6fe to 0afafc1 Compare September 6, 2022 16:02
@neutrinoceros
Copy link
Collaborator Author

@1313e I opened a bit early, right now even import cmasher is broken 😅
Don't worry about allowing CI to run, I'll iterate on this locally and remove the draft status when I'm confident this works as intended

@neutrinoceros neutrinoceros force-pushed the mpl_future_proofing branch 2 times, most recently from c6b45da to cfb6082 Compare September 6, 2022 16:28
@neutrinoceros neutrinoceros marked this pull request as ready for review September 6, 2022 18:36
@neutrinoceros
Copy link
Collaborator Author

@1313e this is now ready for review.

neutrinoceros
neutrinoceros commented Sep 6, 2022
View reviewed changes
cmasher/tests/test_utils.py
cmap_new = mplcm.get_cmap('cmr.rainforest')
assert cmap_new is mod.cmap
cmap_new = _get_cmap('cmr.rainforest')
# assert cmap_new is mod.cmap
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not figure out a way to keep this working, but it also seemed like an implementation detail far from what end-users actually observe. Not sure what is the right approach here

neutrinoceros
neutrinoceros commented Sep 6, 2022
View reviewed changes
cmasher/tests/test_utils.py
cmap_new = mplcm.get_cmap('cmr.infinity')
assert cmap_new is mod.cmap
cmap_new = _get_cmap('cmr.infinity')
# assert cmap_new is mod.cmap
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@neutrinoceros
Copy link
Collaborator Author

I think cmasher's CI is also broken for a different reason, and flake8 tests do not work with the latest version. I was able to pass tests locally using flake8<5

@1313e
Copy link
Owner

1313e commented Sep 6, 2022

@neutrinoceros Thank you.
I will write a reminder to review it this weekend and see about the last few things.

@neutrinoceros
Copy link
Collaborator Author

about flake8 + pytest, this issue seems relevant: tholo/pytest-flake8#87

@1313e 1313e changed the base branch from master to mpl_future_proofing September 17, 2022 09:01
@1313e 1313e merged commit 18d50f4 into 1313e:mpl_future_proofing Sep 17, 2022
@1313e
Copy link
Owner

1313e commented Sep 17, 2022

Thanks for your efforts @neutrinoceros
There are a few extra changes I will have to make, so I merged it into a branch of my own that will soon make its way to master.

@neutrinoceros
Copy link
Collaborator Author

Thank you !

@neutrinoceros neutrinoceros deleted the mpl_future_proofing branch September 17, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Future incompatibility with MPL
2 participants
@neutrinoceros @1313e