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

chore: bump Python version #980

Merged
merged 18 commits into from
Oct 11, 2023
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Oct 6, 2023

This PR introduces pyupgrade, bumps the oldest supported Python interpreter from 3.7 to 3.8, and adds support for 3.12 in the metadata.

It also applies a fix for the removal of NumpyArray.__array__.

@agoose77 agoose77 requested a review from jpivarski October 6, 2023 22:20
@agoose77 agoose77 marked this pull request as ready for review October 6, 2023 22:20
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

Yes, 5.1.0 is a good time for Uproot to drop Python 3.7, though this should merge into the agoose77/wip-shape-tracking branch.

Or better yet, a new main-510 branch should be introduced that agoose77/wip-shape-tracking and agoose77/feat!-bump-python-version both merge into. This main-510 will accumulate everything that's going into Uproot 5.1.0 and not anything that will go in later. When 5.1.0 is finally released, main-510 merges into main and disappears. During this transition time, we shouldn't be merging anything big into main, either.

I'm in principle in favor of pyupgrade, but if it's going to make a lot of small changes across the codebase, we should not introduce it until after

@lobis
Copy link
Collaborator

lobis commented Oct 6, 2023

I guess we should also remove python 3.7 from the GitHub Actions workflow file right?

@jpivarski
Copy link
Member

I guess we should also remove python 3.7 from the GitHub Actions workflow file right?

Good point! That would go with this PR. (Go ahead and make the modification directly; we won't merge it until everyone's satisfied with it.)

@agoose77
Copy link
Collaborator Author

agoose77 commented Oct 7, 2023

@lobis that's weird — I thought I had done that here ... I think I'm working on too many PRs at once 😆 Thanks!

@lobis
Copy link
Collaborator

lobis commented Oct 7, 2023

Should we also add python 3.12 to the CI? (I tried doing so but found some problems, it's a bit hard to debug with the current status of the pipeline, but I think it should be an easy fix).

@agoose77
Copy link
Collaborator Author

agoose77 commented Oct 8, 2023

Should we also add python 3.12 to the CI?

Definitely!

@agoose77 agoose77 force-pushed the agoose77/feat!-bump-python-version branch from 05cabdd to cba9d77 Compare October 11, 2023 07:55
@agoose77 agoose77 enabled auto-merge (squash) October 11, 2023 09:45
@agoose77 agoose77 disabled auto-merge October 11, 2023 10:25
@agoose77
Copy link
Collaborator Author

agoose77 commented Oct 11, 2023

@jpivarski this is ready to merge. It updates our Python min, max versions (>=3.8, <=3.12), and fixes a use of np.asarray(content) that is broken by the latest awkward.

Upon merging, you'd need to update the required branch protection rules.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

I'm merging this into main-v510 and making a new Uproot release candidate from that. These changes require a new minor version.

@jpivarski jpivarski changed the base branch from main to main-v510 October 11, 2023 13:16
@jpivarski jpivarski merged commit 808baf7 into main-v510 Oct 11, 2023
1 of 19 checks passed
@jpivarski jpivarski deleted the agoose77/feat!-bump-python-version branch October 11, 2023 13:50
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.

3 participants