FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

TST/DEP: simplify devdeps requirements by neutrinoceros · Pull Request #20179 · astropy/astropy · GitHub

/ astropy Public

TST/DEP: simplify devdeps requirements - #20179

Draft
neutrinoceros wants to merge 3 commits into
astropy:mainfrom
neutrinoceros:tst/dep/simplify-devdeps
ConversationCommits (3)ChecksFiles changed
Draft

TST/DEP: simplify devdeps requirements#20179
neutrinoceros wants to merge 3 commits into
astropy:mainfrom
neutrinoceros:tst/dep/simplify-devdeps

Conversation

neutrinoceros commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

  • allowing pre-releases for select packages is redundant with the global setting pip_pre=true
  • requiring scipy + matplotlib is an almost complete subset of what the recommended extra already does. It's both simpler and slightly more useful, as well as future-proof to require the extra instead
  • extension-helpers is a build-time dependency, not a runtime one. Installing it from source in the runtime env has no practical effect so it's misleading to require it in this env
  • 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 added this to the v8.1.0 milestone Jul 31, 2026
neutrinoceros added testing no-changelog-entry-needed dependencies Pull requests that update a dependency file labels Jul 31, 2026

github-actions Bot commented Jul 31, 2026

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.

neutrinoceros force-pushed the tst/dep/simplify-devdeps branch from 2d670e7 to 605cfee Compare July 31, 2026 11:26
neutrinoceros added the Extra CI Run cron CI as part of PR label Jul 31, 2026
neutrinoceros marked this pull request as ready for review July 31, 2026 12:37

pllim commented Aug 3, 2026

Copy link
Copy Markdown
Member

Does this have side effects for devs using tox to test locally?

neutrinoceros commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

None intended. This is purely housecleaning.

pllim requested changes Aug 3, 2026
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think this works correctly. I see stable numpy being installed in devdeps with and without scipy in weekly cron (https://github.com/astropy/astropy/actions/runs/30627593365/job/91146325980?pr=20179)

pllim commented Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Looks like it is such on main too. 😱 https://github.com/astropy/astropy/actions/runs/30793237441/job/91621057073

I think we need to understand this problem before we proceed. See #20196

pllim mentioned this pull request Aug 3, 2026
Closed

neutrinoceros commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Now that I figured out #20196, I think it's safe to say that it is completely orthogonal. Can we treat it as such and not block one on the other ?

pllim force-pushed the tst/dep/simplify-devdeps branch from d9782e3 to 3057c2b Compare August 4, 2026 15:43

pllim commented Aug 4, 2026

Copy link
Copy Markdown
Member

I want a apple to apple log comparison, so I rebased for you. Thanks for your patience!

pllim reviewed Aug 4, 2026
View reviewed changes
Comment thread tox.ini
# https://github.com/tox-dev/tox/issues/3433
# But we cannot use alldeps because it messed up oldest-deps pins.
devdeps-!noscipy: objgraph>=1.6.0
devdeps-!noscipy: skyfield>=1.20

pllim Aug 4, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't see skyfield installed in https://github.com/astropy/astropy/actions/runs/30925598933/job/92046974314?pr=20179

I don't think we can remove this.

pllim Aug 4, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

On main, even though numpy dev refuses to install, you can see that it still installed skyfield==1.54

https://github.com/astropy/astropy/actions/runs/30793237441/job/91621057073

neutrinoceros Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

it is part of the test_all extra though

neutrinoceros Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't have an explanation for this quirk off hand. I suspect a bug in tox.

pllim Aug 4, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think it is in the comments above.

neutrinoceros Aug 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

if so, that means the linked report was closed prematurely 😵‍💫

neutrinoceros commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Since not all of this works as intended, I'll try to split this PR into smaller ones.

neutrinoceros marked this pull request as draft August 5, 2026 06:57
neutrinoceros force-pushed the tst/dep/simplify-devdeps branch from 3057c2b to bc473f6 Compare August 5, 2026 07:05

neutrinoceros commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Actually let me try one last thing here

neutrinoceros commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

A couple jobs just failed on network errors while fetching from PyPI. I'll power-cycle this PR in a couple hours.

neutrinoceros reopened this Aug 5, 2026
neutrinoceros reopened this Aug 5, 2026
neutrinoceros force-pushed the tst/dep/simplify-devdeps branch from bc473f6 to 0c1c1ee Compare August 5, 2026 13:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

pllim pllim requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

dependencies Pull requests that update a dependency file Extra CI Run cron CI as part of PR no-changelog-entry-needed testing

Projects

None yet

Milestone

v8.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL