Skip to content

Unpin pytest-astropy-header - #12659

Merged
saimn merged 1 commit into
astropy:mainfrom
pllim:unpin-pytest-astropy-header
Mar 10, 2022
Merged

Unpin pytest-astropy-header#12659
saimn merged 1 commit into
astropy:mainfrom
pllim:unpin-pytest-astropy-header

Conversation

@pllim

@pllim pllim commented Dec 29, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This pull request is an attempt to investigate and then hopefully fix incompatibility between astropy and pytest-astropy-header 0.2.0.

  • When we get to a point where normal CI passes, also enable Extra CI.
  • Remove temporary commit.
  • Wait for upstream fix?

Fixes #12646

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. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
  • 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 this to the v5.0.1 milestone Dec 29, 2021
@github-actions

github-actions Bot commented Dec 29, 2021

Copy link
Copy Markdown
Contributor

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@pllim pllim mentioned this pull request Dec 29, 2021
Closed
@pllim
pllim force-pushed the unpin-pytest-astropy-header branch 2 times, most recently from 27e11f8 to a5ba7b6 Compare December 29, 2021 17:44
@saimn saimn mentioned this pull request Dec 29, 2021
Closed
9 tasks
mhvk
mhvk reviewed Dec 29, 2021
View reviewed changes
Comment thread tox.ini Resolved
@pllim pllim added the Extra CI Run cron CI as part of PR label Dec 29, 2021
@astrofrog astrofrog modified the milestones: v5.0.1, v5.0.2 Jan 27, 2022
@eerovaher

eerovaher commented Mar 3, 2022

Copy link
Copy Markdown
Member

@plim, do you think it's worth seeing if the previously failing tests succeed now?

@pllim
pllim force-pushed the unpin-pytest-astropy-header branch from a5ba7b6 to f36e5a5 Compare March 3, 2022 19:31
@pllim

pllim commented Mar 3, 2022

Copy link
Copy Markdown
Member Author

@eerovaher , still the same problem.

@eerovaher

eerovaher commented Mar 3, 2022

Copy link
Copy Markdown
Member

It would be good to pin pytest-astropy-header>=0.2 because using an older version is what is causing two deprecation warnings to appear in the CI tests, although those warnings do not lead to test failures. But if the tests are failing here then we are stuck with those warnings.

@pllim

pllim commented Mar 3, 2022

Copy link
Copy Markdown
Member Author

Yeah... I don't have time to investigate why. Simon tried and gave up.

A hack is to add back the import check for astropy back in pytest-astropy-header that Simon doesn't like.

@saimn

saimn commented Mar 3, 2022

Copy link
Copy Markdown
Contributor

Well the problem is that the current setup to test the installed code only works because pytest-astropy-header<0.2 is importing astropy before pytest run the tests.

@pllim
pllim force-pushed the unpin-pytest-astropy-header branch from f36e5a5 to d408c26 Compare March 4, 2022 20:00
@pllim pllim mentioned this pull request Mar 4, 2022
Merged
1 task done
@pllim

This comment was marked as outdated.

Sign in to view
@pllim
pllim force-pushed the unpin-pytest-astropy-header branch from b77c0e3 to a8177c2 Compare March 10, 2022 01:49
@pllim
pllim marked this pull request as ready for review March 10, 2022 03:28
@pllim

pllim commented Mar 10, 2022

Copy link
Copy Markdown
Member Author

No more warnings. 😸

========= 22885 passed, 217 skipped, 146 xfailed in 449.56s (0:07:29) ==========
___________________________________ summary ____________________________________
  py39-test-alldeps: commands succeeded
  congratulations :)

@pllim
pllim requested review from eerovaher and saimn March 10, 2022 03:30
eerovaher
eerovaher reviewed Mar 10, 2022
View reviewed changes
Comment thread setup.cfg Resolved
eerovaher
eerovaher reviewed Mar 10, 2022
View reviewed changes
Comment thread setup.cfg Resolved
@pllim
Update pytest-astropy-header pin
da084a1 
to a compatible version.
@pllim
pllim force-pushed the unpin-pytest-astropy-header branch from a8177c2 to da084a1 Compare March 10, 2022 17:41
@saimn

saimn commented Mar 10, 2022

Copy link
Copy Markdown
Contributor

We need a 5.0.3 milestone :)

@saimn saimn modified the milestones: v5.0.2, v5.0.3 Mar 10, 2022
saimn
saimn approved these changes Mar 10, 2022
View reviewed changes

@saimn saimn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good to me (and I could create the 5.0.3 milestone so let's merge !). Thanks @pllim.

@saimn
saimn merged commit 97be610 into astropy:main Mar 10, 2022
@lumberbot-app

lumberbot-app Bot commented Mar 10, 2022

Copy link
Copy Markdown

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 v5.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -m1 97be610d13cf314b28ffe8eddc20f9affa8a05ca
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #12659: Unpin pytest-astropy-header'
  1. Push to a named branch:
git push YOURFORK v5.0.x:auto-backport-of-pr-12659-on-v5.0.x
  1. Create a PR against branch v5.0.x, I would have named this PR:

"Backport PR #12659 on branch v5.0.x (Unpin pytest-astropy-header)"

And apply the correct labels and milestones.

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

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

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

@pllim
pllim deleted the unpin-pytest-astropy-header branch March 10, 2022 21:40
@pllim

pllim commented Mar 10, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Thanks, all! I'll do the manual backport (#12947).

pllim added a commit to pllim/astropy that referenced this pull request Mar 10, 2022
@pllim
Backport PR astropy#12659: Unpin pytest-astropy-header
aa20f5a
@pllim pllim mentioned this pull request Mar 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk left review comments
@eerovaher eerovaher eerovaher left review comments
@saimn saimn saimn approved these changes

Assignees

No one assigned

Labels

Extra CI Run cron CI as part of PR installation no-changelog-entry-needed Ready-for-final-review testing

Projects

None yet

Milestone

v5.0.3

Development

Successfully merging this pull request may close these issues.

MNT: Unpin pytest-astropy-header when 0.2.1 or later is released

5 participants

@pllim @eerovaher @saimn @mhvk @astrofrog