-
Notifications
You must be signed in to change notification settings - Fork 31
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
Drop support for awkward v1 #434
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #434 +/- ##
==========================================
+ Coverage 82.77% 82.79% +0.02%
==========================================
Files 96 96
Lines 11583 11575 -8
==========================================
- Hits 9588 9584 -4
+ Misses 1995 1991 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, but you want to be careful about when you merge it. When you merge it in and publish a release 1.4.0, there will be a dividing line: users of Awkward 1 (such as users of Coffea 0.7) will be more restricted in which Vector version they can take. Ideally, this should happen in a relatively quiet time when there aren't other important changes going in, so that someone isn't forced to switch Awkward versions because they need a small Vector feature that went in after 1.4.0. In cases like that, they'd probably ask for the new Vector feature to be back-ported to 1.3.x. Doing this during a busy time would mean adding the extra work of back-porting to that work.
Is it a relatively quiet time now? The Coffea-Vector synchronization that's going on now, is that strictly Coffea 2024 (not 0.7), which is already Awkward 2 so the above wouldn't be an issue?
Also, immediately before merging this PR, make a branch off of main
called main-1.3
, which will make it easier to back-port. Back-port PRs would merge into main-1.3
, rather than main
.
Thanks for the review! That makes sense, I'll create a new branch before merging the PR. I think I'll hold this till coffea migrates to vector as the process has already changed several things in vector. |
1bf3206
to
ec24efd
Compare
619442a
to
a4bc491
Compare
a4bc491
to
99022eb
Compare
Now that coffea has switched to vector and the changes are up in a release, it should be okay to merge this in. I will wait for a couple of weeks before merging this, just in case we get some suggestions from coffee users. I think pyhep.dev would be the perfect place to merge this in and create a new release. |
Description
Kindly take a look at CONTRIBUTING.md.
Please describe the purpose of this pull request. Reference and link to any relevant issues or pull requests.
Checklist
$ pre-commit run --all-files
or$ nox -s lint
)$ pytest
or$ nox -s tests
)$ cd docs; make clean; make html
or$ nox -s docs
)$ pytest --doctest-plus src/vector/
or$ nox -s doctests
)Before Merging