TST: remove workarounds now that contourpy has unpinned nightlies by neutrinoceros · Pull Request #16122 · astropy/astropy

neutrinoceros · 2024-02-28T09:01:37Z

Description

This reverts ~~#15290~~ (and parts of #15363) following contourpy/contourpy#362

EDIT: xref matplotlib/matplotlib#26847 (comment)

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.

github-actions · 2024-02-28T09:02:04Z

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 · 2024-02-28T09:31:12Z

hum, I must be doing something wrong: I keep breaking the workflows this is supposed to simplify. I'm wondering if I should just leave it be (don't fix what ain't broke).

neutrinoceros · 2024-02-28T18:07:09Z

Please feel free to take the wheel here ! but we can reassess after #15603 goes in too. In any case this doesn't seem too urgent :)

pllim · 2024-02-28T18:48:15Z

take the wheel

I see what you did there! 👏

matthewfeickert · 2024-02-29T02:23:26Z

    devdeps: python -I -m pip install -v --pre

 commands =
+    mpldev: pip install -U --pre -i https://pypi.anaconda.org/scientific-python-nightly-wheels/simple matplotlib


👋 As noted in the CI logs (https://github.com/astropy/astropy/actions/runs/8078075132/job/22069637834?pr=16122#step:10:33) as there are additional dependencies that don't live on the index you'll need to include the PyPI as the --extra-index-url. c.f. the example in the scientific-python/upload-nightly-action README

Suggested change

mpldev: pip install -U --pre -i https://pypi.anaconda.org/scientific-python-nightly-wheels/simple matplotlib

mpldev: pip install -U --pre --index-url https://pypi.anaconda.org/scientific-python-nightly-wheels/simple --extra-index-url https://pypi.org/simple matplotlib

Oh, that's it ! I got confused while cherry-picking parts to revert. PIP_EXTRA_INDEX_URL is already set on line 25, but I was indeed effectively ignoring it here. Thanks !

I'd still in general recommend switching the indexes (to --index-url https://pypi.anaconda.org/scientific-python-nightly-wheels/simple --extra-index-url https://pypi.org/simple) to get in the habit of helping guard against what happened to PyTorch in 2022. But in this particular case it doesn't matter much (or actually at all), given that the name of the packges exist already on both indexes and you are trusting scientific-python to not do anything shady on our index.

wow, thanks for sharing ! Happy to update this patch if @pllim feels we should

I think you're fine. Just an FYI for the future when you're interacting with other potential indexes. 👍

Revisiting this comment now... The line in the suggestion no longer exists. In tox, we define PIP_EXTRA_INDEX_URL in setenv, following by matplotlib>=0.0.dev0 in deps. Not sure if I want to mess with --index-url now especially that this is not a concern right now. Thanks anyway!

pllim

~~Does not hurt to have the extra index guard as suggested in this PR.~~ (Changed my mind, see #16122 (comment))

But we agreed that this is not urgent, so I can take over when the Python 3.10 PR is merged. Thanks, all!

while retaining mpldev for image tests.

pllim · 2024-03-08T22:22:32Z

To answer my own question, no we do not need to explicitly pull in contourpy-dev because mpl-dev magically does that.

pllim · 2024-03-08T22:23:40Z

I also want to run the cron jobs, especially the "no scipy" one, just in case. When I am happy with this, I will approve and merge.

Thanks!

pllim

Logs look fine. Linkheck failure unrelated. Exotic archs shouldn't matter here, so I cancelled them because they take too long to run.

Thanks again!

neutrinoceros · 2024-03-09T05:50:42Z

Thank you for finishing this !

…6122) * TST: remove workarounds now that contourpy has unpinned nightlies * TST: Recombine mpldev into devdeps CI job while retaining mpldev for image tests. --------- Co-authored-by: P. L. Lim <[email protected]>

github-actions Bot added the testing label Feb 28, 2024

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

pllim added this to the v6.1.0 milestone Feb 28, 2024

pllim added no-changelog-entry-needed numpy-dev labels Feb 28, 2024

matthewfeickert suggested changes Feb 29, 2024

View reviewed changes

neutrinoceros force-pushed the tests/simplify_devdeps_install branch from 9209ea9 to f952f6d Compare February 29, 2024 06:46

neutrinoceros marked this pull request as ready for review February 29, 2024 07:04

pllim reviewed Feb 29, 2024

View reviewed changes

Comment thread tox.ini Outdated

Comment thread tox.ini Outdated

TST: remove workarounds now that contourpy has unpinned nightlies

71fafaa

pllim force-pushed the tests/simplify_devdeps_install branch 2 times, most recently from eb970c4 to 360237b Compare March 8, 2024 21:33

This comment was marked as resolved.

Sign in to view

TST: Recombine mpldev into devdeps CI job

d4becdf

while retaining mpldev for image tests.

pllim force-pushed the tests/simplify_devdeps_install branch from 675e3d7 to d4becdf Compare March 8, 2024 22:10

pllim added the Extra CI Run cron CI as part of PR label Mar 8, 2024

pllim approved these changes Mar 8, 2024

View reviewed changes

pllim merged commit 3521b0a into astropy:main Mar 8, 2024

neutrinoceros deleted the tests/simplify_devdeps_install branch March 9, 2024 05:50

	mpldev: pip install -U --pre -i https://pypi.anaconda.org/scientific-python-nightly-wheels/simple matplotlib
	mpldev: pip install -U --pre --index-url https://pypi.anaconda.org/scientific-python-nightly-wheels/simple --extra-index-url https://pypi.org/simple matplotlib

Uh oh!

Conversation

neutrinoceros commented Feb 28, 2024 • edited by pllim Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions Bot commented Feb 28, 2024

Uh oh!

This comment was marked as outdated.

neutrinoceros commented Feb 28, 2024

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

neutrinoceros commented Feb 28, 2024

Uh oh!

pllim commented Feb 28, 2024

Uh oh!

matthewfeickert Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Feb 29, 2024

Choose a reason for hiding this comment

Uh oh!

matthewfeickert Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim Mar 8, 2024

Choose a reason for hiding this comment

Uh oh!

pllim left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

pllim commented Mar 8, 2024

Uh oh!

pllim commented Mar 8, 2024

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

neutrinoceros commented Mar 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

neutrinoceros commented Feb 28, 2024 •

edited by pllim

Loading

matthewfeickert Feb 29, 2024 •

edited

Loading

matthewfeickert Feb 29, 2024 •

edited

Loading

pllim left a comment •

edited

Loading