Skip to content

TST/DEP: drop pytest-astropy in favor of corresponding, direct test dependencies - #18784

Merged
pllim merged 1 commit into
astropy:mainfrom
neutrinoceros:tst/drop-pytest-astropy
Jul 29, 2026
Merged

pllim merged 1 commit into
astropy:mainfrom
neutrinoceros:tst/drop-pytest-astropy

Conversation

@neutrinoceros

@neutrinoceros neutrinoceros commented Oct 26, 2025 •

Copy link
Copy Markdown
Contributor

Description

Based of #18780

fix #18783

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@neutrinoceros neutrinoceros added this to the v7.2.0 milestone Oct 26, 2025
@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • 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 instructions for rebase and squash.
  • 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. Codestyle issues can be fixed by the bot.
  • 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 this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • 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.

@github-actions

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?

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

Interesting. Looks like the --run-slow flag (and corresponding @pytest.mark.slow marker) are defined in some plugin that's pulled from pytest-astropy, but I haven't been able to identified which one yet.

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

of course, it's from pytest-astropy itself.

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

6804a6_2a566775e2464e6884a9bcab4c3060f8_mv2 gif_w=500

@pllim

pllim commented Oct 28, 2025

Copy link
Copy Markdown
Member

@pllim

pllim commented Oct 28, 2025

Copy link
Copy Markdown
Member

So yeah I am not sure if this is feasible without refactoring pytest-astropy itself... Maybe open discussion on astropy-dev if you still want to pursue?

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

I do want to continue experimenting with this, but I don't think the problems I'm after can be addressed from pytest-astropy itself without breaking its users.

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

(and with the impeding feature freeze, I'll have to postpone this work until at least next week)

@astrofrog astrofrog modified the milestones: v7.2.0, v8.0.0 Nov 3, 2025
@neutrinoceros
neutrinoceros force-pushed the tst/drop-pytest-astropy branch from 2e23c32 to ca53c65 Compare November 11, 2025 08:36
@neutrinoceros

Copy link
Copy Markdown
Contributor Author

Turns out vendoring the contents of that plugin within our root conftest.py is actually trivial. I just suspected it could not be, which is why I didn't prioritize this earlier.
Any way this should be much more stable now. Note that it's still based of #18780 though, but I think I could avoid that coupling. I'll try this next once CI is clear.

@neutrinoceros
neutrinoceros force-pushed the tst/drop-pytest-astropy branch from ca53c65 to 3924e50 Compare November 11, 2025 08:50
@neutrinoceros neutrinoceros added the Extra CI Run cron CI as part of PR label Nov 11, 2025
@neutrinoceros

Copy link
Copy Markdown
Contributor Author

I'm seeing a couple unrelated failures caused by #18880

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

other failures I'm seeing are already handled in existing PRs

@pllim

pllim commented Nov 11, 2025

Copy link
Copy Markdown
Member

The challenge isn't code, but people. We wanted the plugin stuff in pytest-astropy so other packages could also use it without requiring astropy, but they were so trivial that we did not want to create a whole new package just for them.

Now, they been upstream long enough that we probably have a number of packages using them, and have to keep maintaining them. So copying them over is just duplicating code.

This definitely need more discussion.

@neutrinoceros

neutrinoceros commented Nov 11, 2025 •

Copy link
Copy Markdown
Contributor Author

we could easily make these markers into their own plugin if duplication must be avoided, but for what it's worth:

  • duplicating ~50 lines of code seems like a (much) lesser evil than irreproducible builds
  • I don't think we have to make this change throughout the entire ecosystem; it's diminishing returns from here, but astropy being a core package means (to me) it needs to adhere to strict(er) quality requirements

@pllim

pllim commented Nov 11, 2025

Copy link
Copy Markdown
Member

Moving them anywhere now would create churn. We have to find and inform all the affected downstream packages. And then go through deprecation period and face some resistance. And then still keep them going while deprecated. And then finally we have to remove them.

@neutrinoceros

neutrinoceros commented Nov 11, 2025 •

Copy link
Copy Markdown
Contributor Author

We have to find and inform all the affected downstream packages.

why ? couldn't we just keep pytest-astropy as the "batteries included" option, even if we stop dog-fooding it in astropy itself ? I glanced over it and it didn't seem too burdensome to maintain (no release for 2 years and it's still working for everyone, as far as I can tell). Am I missing anything ?

@pllim

pllim commented Nov 11, 2025

Copy link
Copy Markdown
Member

So you are saying we create a new package for the plugin and then include that in pytest-astropy, instead of moving it here?

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

Not really. I would prefer that the code we use to test the core library live in the same repo, which is what the current state of the branch does, and leave pytest-astropy live its life as the provider of these functionality for everyone else that choose to adopt it.

But if we must avoid code duplication at all cost, there are other avenues. Though, it seems we agree that they are highly disruptive and undesirable.

@neutrinoceros
neutrinoceros force-pushed the tst/drop-pytest-astropy branch from 06f3f82 to 04e2f84 Compare July 24, 2026 19:15
@neutrinoceros

Copy link
Copy Markdown
Contributor Author

fixed

@pllim
pllim force-pushed the tst/drop-pytest-astropy branch from 04e2f84 to fc9eaae Compare July 28, 2026 20:59
@neutrinoceros
neutrinoceros force-pushed the tst/drop-pytest-astropy branch from fc9eaae to fa9c7c2 Compare July 29, 2026 14:27
@neutrinoceros
neutrinoceros force-pushed the tst/drop-pytest-astropy branch from fa9c7c2 to 00536b8 Compare July 29, 2026 14:28
Comment thread tox.ini
devpytest: git+https://github.com/astropy/pytest-arraydiff.git
devpytest: git+https://github.com/astropy/pytest-filter-subpackage.git
devpytest: git+https://github.com/matplotlib/pytest-mpl.git
devpytest: git+https://github.com/astropy/pytest-astropy.git

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess we could now pull in dev versions of pytest-astropy-header, pytest-cov, and pytest-remotedata, but previously it was just pulling in dev version of the metapackage which does not mean the same thing, so we can do that as a follow-up if we really want to.

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@pllim
pllim merged commit 11e9073 into astropy:main Jul 29, 2026
43 of 45 checks passed
@neutrinoceros
neutrinoceros deleted the tst/drop-pytest-astropy branch July 29, 2026 17:38
@pllim

pllim commented Aug 4, 2026 •

Copy link
Copy Markdown
Member

Might have to manually backport some of this and #20165

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

how so ? I feel like I missed something

@pllim

pllim commented Aug 4, 2026

Copy link
Copy Markdown
Member

#20201 (comment)

@neutrinoceros

Copy link
Copy Markdown
Contributor Author

oh, indeed. I can prep manual backports now.

@neutrinoceros neutrinoceros modified the milestones: v8.1.0, v7.2.3 Aug 4, 2026
@neutrinoceros neutrinoceros added backport-v7.2.x on-merge: backport to v7.2.x backport-v8.0.x on-merge: backport to v8.0.x and removed installation labels Aug 4, 2026
neutrinoceros pushed a commit to neutrinoceros/astropy that referenced this pull request Aug 4, 2026
…stropy

TST/DEP: drop pytest-astropy in favor of corresponding, direct test dependencies
pllim added a commit that referenced this pull request Aug 4, 2026
TST/DEP: drop pytest-astropy in favor of corresponding, direct test dependencies
pllim added a commit that referenced this pull request Aug 4, 2026
Backport PR #18784 on branch v8.0.x (TST/DEP: drop pytest-astropy in favor of corresponding, direct test dependencies)
pllim added a commit that referenced this pull request Aug 5, 2026
Backport PR #18784 on branch v7.2.x (TST/DEP: drop pytest-astropy in favor of corresponding, direct test dependencies)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-v7.2.x on-merge: backport to v7.2.x backport-v8.0.x on-merge: backport to v8.0.x dependencies Pull requests that update a dependency file dev-automation Extra CI Run cron CI as part of PR keep-open no-changelog-entry-needed testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TST/DEP: replace pytest-astropy with corresponding, direct dependencies

4 participants