Skip to content

TST: Use proper matplotlib dev check#12059

Merged
pllim merged 1 commit intoastropy:mainfrom
pllim:mpl-dev-check
Aug 16, 2021
Merged

TST: Use proper matplotlib dev check#12059
pllim merged 1 commit intoastropy:mainfrom
pllim:mpl-dev-check

Conversation

@pllim
Copy link
Copy Markdown
Member

@pllim pllim commented Aug 16, 2021 •
edited
Loading

Description

This pull request is to use proper check logic for matplotlib dev/stable version check, made possible by matplotlib/matplotlib#19419 (which was milestoned to 3.5.0). If some of our jobs fail, this PR might have to wait till the minversion of matplotlib can use this feature.

With this patch, we no longer have to bump the matplotlib version check in this file after every matplotlib release, as we had to do in the past in #11717 and #12052.

Fixes #12055

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the Extra CI label.
  • Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label.
  • Is a milestone set? Milestone must be set but astropy-bot check might be missing; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate backport-X.Y.x label(s) before merge.

@pllim pllim added no-changelog-entry-needed Extra CI Run cron CI as part of PR labels Aug 16, 2021
@pllim pllim added this to the v5.0 milestone Aug 16, 2021
@pllim pllim added the testing label Aug 16, 2021
@pllim pllim requested review from dhomeier, mhvk and nstarman August 16, 2021 16:47
nstarman
nstarman reviewed Aug 16, 2021
View reviewed changes
astropy/visualization/wcsaxes/tests/test_misc.py
TEX_UNAVAILABLE = not matplotlib.checkdep_usetex(True)

MATPLOTLIB_GT_3_4_3 = Version(matplotlib.__version__) > Version('3.4.3 ')
MATPLOTLIB_DEV = Version(matplotlib.__version__).is_devrelease
Copy link
Copy Markdown
Member

@nstarman nstarman Aug 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@pllim, you're more familiar with the history of this problem. Are only dev releases problematic, or have pre-releases likewise had dpi attribute weirdness? Looking at
https://packaging.pypa.io/en/latest/version.html#packaging.version.Version.is_prerelease
I see there's a flag .is_prerelease that is True for both pre and dev releases.

Copy link
Copy Markdown
Member Author

@pllim pllim Aug 16, 2021

Choose a reason for hiding this comment

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

Not sure. But we never test prerelease in the CI. If you look at tox.ini, it either installs stable versions or from source. 🤷

Copy link
Copy Markdown
Member

@nstarman nstarman Aug 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Fair enough. Can always be adjusted later if we ever test on pre-releases.

@nstarman nstarman added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Aug 16, 2021
@nstarman
Copy link
Copy Markdown
Member

nstarman commented Aug 16, 2021

It looks like me adding the "merge-when-ci-passes" label retriggered the daily and weekly CRON jobs. That's probably not supposed to happed 😬

@pllim
Copy link
Copy Markdown
Member Author

pllim commented Aug 16, 2021

Ops... @nstarman , can you please open an issue about the labeling CI bug? I will just cancel this rerun. There is no point to run them again.

@pllim pllim merged commit f1fdd58 into astropy:main Aug 16, 2021
@pllim pllim deleted the mpl-dev-check branch August 16, 2021 17:28
@pllim
Copy link
Copy Markdown
Member Author

pllim commented Aug 16, 2021

Thanks for the quick review!

@pllim pllim mentioned this pull request Aug 17, 2021
Closed
@pllim pllim modified the milestones: v5.0, v4.3.2 Aug 17, 2021
@pllim
Copy link
Copy Markdown
Member Author

pllim commented Aug 17, 2021

@meeseeksdev backport to v4.3.x

@lumberbot-app
Copy link
Copy Markdown

lumberbot-app bot commented Aug 17, 2021

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v4.3.x
$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 f1fdd58f78a13e36cbeb9fb4449478b8987700ba
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am 'Backport PR #12059: TST: Use proper matplotlib dev check'
  1. Push to a named branch :
git push YOURFORK v4.3.x:auto-backport-of-pr-12059-on-v4.3.x
  1. Create a PR against branch v4.3.x, I would have named this PR:

"Backport PR #12059 on branch v4.3.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

pllim added a commit that referenced this pull request Aug 17, 2021
@pllim
Manual backport of #12059
c16c982
@pllim
Copy link
Copy Markdown
Member Author

pllim commented Aug 17, 2021

Manual backport to v4.3.x done in c16c982 . I don't see this failure in LTS.

astrofrog pushed a commit that referenced this pull request Aug 18, 2021
@pllim @astrofrog
Merge pull request #12059 from pllim/mpl-dev-check
4437aad 
TST: Use proper matplotlib dev check
@astrofrog
Copy link
Copy Markdown
Member

astrofrog commented Aug 19, 2021

I had to backport this to LTS in the end so updating the milestone

@astrofrog astrofrog modified the milestones: v4.3.2, v4.0.6 Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nstarman nstarman nstarman approved these changes

@dhomeier dhomeier Awaiting requested review from dhomeier

@mhvk mhvk Awaiting requested review from mhvk

Assignees

No one assigned

Labels

Extra CI Run cron CI as part of PR no-changelog-entry-needed Ready-for-final-review testing visualization.wcsaxes zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now.

Projects

None yet

Milestone

v4.0.6

Development

Successfully merging this pull request may close these issues.

TST: Check for matplotlib dev version properly

4 participants

@pllim @nstarman @astrofrog @saimn