Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated continuous integration #38

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Conversation

astrofrog
Copy link
Member

@astrofrog astrofrog commented Sep 11, 2023

Just simplifying the CI and adding recent versions of pytest

Fix #39

@pllim
Copy link
Member

pllim commented Sep 11, 2023

Should we just drop pytest<6?

@pllim
Copy link
Member

pllim commented Sep 11, 2023

CI disabled. Gonna close/reopen to trigger it.

@pllim pllim closed this Sep 11, 2023
@pllim pllim reopened this Sep 11, 2023
@astrofrog astrofrog changed the title Fixes for recent versions of pytest Updated continuous integration Sep 11, 2023
@astrofrog
Copy link
Member Author

@pllim - I don't think we need to drop any pytest versions at this point, think it should all work

@pllim
Copy link
Member

pllim commented Sep 11, 2023

You wanna be adventurous and have pytestdev use Python 3.12? 😅

@astrofrog
Copy link
Member Author

Does Python 3.12 work with the openastronomy workflows?

@pllim
Copy link
Member

pllim commented Sep 11, 2023

I have no idea. I just know @bsipocz added a Python 3.12 job for pytest-doctestplus (and I don't have time to check right now).

@astrofrog
Copy link
Member Author

astrofrog commented Sep 11, 2023

@pllim lets just merge this for now if you are happy with it and I can investigate separately?

@astrofrog astrofrog marked this pull request as ready for review September 11, 2023 13:56
- macos: py310-test-pytest73
- windows: py311-test-pytest74
- linux: py311-test-pytestdev
publish:
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Can we still use the "Build wheels" label on PRs here?

Copy link
Member Author

@astrofrog astrofrog Sep 11, 2023

Choose a reason for hiding this comment

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

The wheels are always built in PRs with this config. It's so fast there's no point making it optional.

Copy link
Member Author

Choose a reason for hiding this comment

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

Wheels will always be built but only published to PyPI on a tag

Copy link
Member

Choose a reason for hiding this comment

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

OK so after we have merged this, we should delete that "Build wheels" label.

@pllim
Copy link
Member

pllim commented Sep 11, 2023

This drops Python 3.7 testing without dropping Python 3.7 support. Should we just drop 3.7 completely?

python_requires = >=3.7

@astrofrog
Copy link
Member Author

Oops yes meant to update setup.cfg

@bsipocz
Copy link
Member

bsipocz commented Sep 11, 2023

IMO, do one last release before dropping 3.7, that's what we did elsewhere. Or is it hopelessly incompatible already?

@bsipocz
Copy link
Member

bsipocz commented Sep 11, 2023

Re py3.12: there wasn't any big issues, but this one fix was needed, which may or may not effect this plugin: scientific-python/pytest-doctestplus#215

@astrofrog
Copy link
Member Author

Can add back python 3.7, not a problem!

@astrofrog
Copy link
Member Author

I've added back Python 3.7, hopefully this should still pass

@astrofrog astrofrog requested a review from pllim September 11, 2023 20:26
tox.ini Outdated Show resolved Hide resolved
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM but I'll let Brigitta have a look too. Thanks!

@pllim pllim requested a review from bsipocz September 11, 2023 22:58
Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

Versions certainly need to be updated, but I don't see why relying on a middle layer makes sense for these super simple plugins.

.github/workflows/ci_workflows.yml Show resolved Hide resolved
uses: OpenAstronomy/github-actions-workflows/.github/workflows/tox.yml@v1
with:
envs: |
- linux: py37-test-pytest46
Copy link
Member

Choose a reason for hiding this comment

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

Somewhat unrelated improvement: instead of pytest46 I like to call it pytestoldest, and pin it to the actual stated minimum version rather than a wildcard in that series. E.g. pytest==4.6.0

If a fix is needed from an actual bugfix, then bump the minimum version to reflect that.

(for the rest of the versions I feel the flexible wildcard is fine, we need a wide enough matrix to cover over versions but don't need to test everything).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, will do

@bsipocz
Copy link
Member

bsipocz commented Sep 11, 2023

Oh, and I can repeat what @pllim said above, please add py3.12 testing (can be done separately, but it's really time to make stuff compatible if it's not yet)

@pllim
Copy link
Member

pllim commented Sep 12, 2023

@astrofrog
Copy link
Member Author

This fails on Python 3.12 - @pllim @bsipocz is that an error you've seen before?

tox.ini Outdated Show resolved Hide resolved
@pllim
Copy link
Member

pllim commented Sep 12, 2023

I don't think you can pull astropy or tables for py312. You also have to pull in pandas dev.

[options.extras_require]
test =
astropy
pandas
tables

@astrofrog
Copy link
Member Author

Hmm at the moment astropy is a required dependency for the test suite. Rather than re-write everything maybe we can just wait?

@pllim
Copy link
Member

pllim commented Sep 12, 2023

tox.ini Outdated Show resolved Hide resolved
Simplified publish workflow

Make publish job dependent on tests

Add back Python 3.7

Rename pytest46 to pytestoldest and add Python 3.12 testing

Use latest developer version of Numpy and pytest for Python 3.12

Co-authored-by: P. L. Lim <[email protected]>
@pllim pllim force-pushed the fix-pytest-72 branch 2 times, most recently from 25b0076 to 98e772b Compare November 15, 2023 22:15
across the board and add the codestyle check.

Fix PEP 8 complaints.

Auto-cancel duplicate Action jobs.
@pllim pllim force-pushed the fix-pytest-72 branch 2 times, most recently from d8a4e31 to beba1cb Compare November 15, 2023 22:31
Handle FutureWarning from pandas 3.0

Skip failing pandas test caused by numpy 2.0

Updated matrix to run a stable job in py312
@pllim
Copy link
Member

pllim commented Nov 15, 2023

Gonna merge and then do a release. 🤞

@pllim pllim merged commit bab9723 into astropy:main Nov 15, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility with NumPy 2.0 and Python 3.12
3 participants