-
Notifications
You must be signed in to change notification settings - Fork 443
Fixes for Matplotlib 3.7 #2927
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
Fixes for Matplotlib 3.7 #2927
Conversation
Matplotlib now triggers default unit-conversion axis labels for axhline/axvline. Our test was not expecting that.
visible/b is implied when other keyword arguments are passed. Also, b= is no longer supported with Matplotlib 3.7.
|
I guess an alternative to modifying the existing flow with reporting is to create a separate workflow that triggers on the label and doesn't have a reporting job. Not sure if that's worth the effort, but open to opinions from others. I also just noticed these jobs are running on Python 3.10, so I'll try kicking these over to 3.11 when I patch this PR up to at least address the lint. |
Passing 3 arguments to set_ylim() was not working as intended (with the third argument being "step"). Instead, pass only two expected arguments to set_ylim (was passing as emit, which ceases to be a positional argument in Matplotlib 3.8) and set an appropriate tick locator instead to work for "step".
|
So it looks like the current iteration requires the label to be reapplied. Not a huge fan, but not exactly wanting to write a bunch of API calls to avoid it... |
Borrowed from one of Matplotlib's workflows.
|
Ok, matplotlib had a version that showed me the right way to do this. |
|
Ok, so refactored so that we have:
|
|
I'm not sure how MetPy/.github/workflows/nightly-builds.yml Lines 56 to 60 in e493f6d
need updated? |
|
Good catch. For one, I mistakenly left a condition on even kicking off the builds. Then it's definitely not requesting the results correctly. Probably gonna need some "prints" to figure out the right syntax here, assuming it's available. |
|
I think we're good to go here now. |
Description Of Changes
meteogram_metpy.pyto adjust for deprecated/removed matplotlib arguments. Was pretty much unneeded.This also now modifies the nightly workflow to trigger on having a label applied to the PR, which will allow us to test PRs like this which need to run outside our standard config in order to validate the fix.
Checklist