feat: migrate to awkward v2 (and keep supporting v1)

[Saransh-cpp]

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

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't any other open Pull Request for the required change ?
• Does your submission pass pre-commit? ($ pre-commit run --all-files or $ nox -s lint)
• Does your submission pass tests? ($ pytest or $ nox -s tests)
• Does the documentation build with your changes? ($ cd docs; make clean; make html or $ nox -s docs)
• Does your submission pass doctests? ($ xdoctest ./src/vector or $ nox -s doctests)

Before Merging

• Summarize commit messages into a brief review of the Pull request.

[Saransh-cpp]

Some comments and questions -

• I have been trying to test vector on the new awkward v2.0.0rc1, but I somehow cannot get around the numba errors. Is there (or will there be) a migration guide for the awkward v1 -> v2 transition? (I decided to push the code here because I could not resolve it locally)
• Should vector strictly support awkward>=2 in future releases (whenever awkward v2 with no "rc" is out)?
• I am not very sure if the numba errors are on the vector side or on the awkward side 😓
• Given that vector v1 is not out yet, should we support awkward v2 (strict?) in vector v1?

cc: @jpivarski @henryiii

Thanks!

[agoose77]

@Saransh-cpp it looks like the bug here is that RecordArrayType renamed recordlookup to fields in v2 :)

[Saransh-cpp]

Thank you so much, @agoose77! I really should start reading some text on debugging general numba failures 😅

[codecov-commenter]

Codecov Report

Base: 81.08% // Head: 83.73% // Increases project coverage by +2.65% 🎉

Coverage data is based on head (d2a83af) compared to base (4d060a9).
Patch coverage: 86.66% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #284      +/-   ##
==========================================
+ Coverage   81.08%   83.73%   +2.65%     
==========================================
  Files          96       96              
  Lines       10672    10682      +10     
==========================================
+ Hits         8653     8945     +292     
+ Misses       2019     1737     -282     

Impacted Files	Coverage Δ
src/vector/__init__.py	80.43% <60.00%> (-2.50%)	⬇️
src/vector/backends/awkward.py	81.68% <92.00%> (+30.04%)	⬆️
src/vector/backends/_numba_object.py	69.18% <0.00%> (+0.11%)	⬆️
src/vector/backends/object.py	75.12% <0.00%> (+1.44%)	⬆️
src/vector/backends/numpy.py	80.00% <0.00%> (+1.89%)	⬆️
src/vector/backends/awkward_constructors.py	57.41% <0.00%> (+12.44%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Saransh-cpp]

Okay, this feels more like awkward v2 support, as a user can now use vector with awkward v2.0.0rc1. Before this, vector was not using awkward._v2 anywhere, instead, it was only tested on awkward._v2.

[Saransh-cpp]

Thanks, @agoose77! Looks much better now!

[agoose77]

Ah, good catch on the type hint.

[agoose77]

Why has this moved? This changes the typing permissiveness. I'm not au-fait with what conventions we follow, maybe @henryiii can check this?

[Saransh-cpp]

Hmm, I initially moved this because it solved the failing "importlib_metadata has no typing stubs" error. I'll move this back!

[agoose77]

This fails for me if I don't install ipykernel, which is included in the docs extra. I'll discuss this in the CI configuration down below.

[Saransh-cpp]

That is weird. This does seem to work in the GH Actions CI 🤔

[agoose77]

I assume we don't run just pytest --xdoctest . anywhere? The docs extra will be bringing in ipykernel. I suppose this isn't an issue though - I just needed to read CONTRIBUTING.md more closely; we already tell the reader to install the docs extra above.

[agoose77]

So, this isn't a problem!

[Saransh-cpp]

Thanks for the review, @agoose77! Looks much better now!

[Saransh-cpp]

awkward>=2.0.0rc1 was somehow not installing anything, but changing it to "awkward>=2.0.0rc1" worked.

For instance, see the "Install awkward v2" step here - https://github.com/scikit-hep/vector/actions/runs/3496655692/jobs/5874212118 (it is empty, in fact, all the "Install awkward v2" steps executed before the last commit are empty).

[agoose77]

Hmm, if awkward wasn't being installed, it should have failed the pip step. I suspect bash was treating the > as a redirect for stdout, which would still have succeeded as we're passing the --pre flag.

[Saransh-cpp]

Weird behaviour, but everything works now!

[Saransh-cpp]

Some discussion points -

• Should vector strictly support awkward>=2 in future releases (whenever awkward v2 with no "rc" is out)?
• Given that vector v1 is not out yet, should we support awkward v2 (strict?) in vector v1?

[jpivarski]

Should vector strictly support awkward>=2 in future releases (whenever awkward v2 with no "rc" is out)?

I think it would be easiest to support only one version. The switchover from requiring awkward>=1 to awkward>=2 should happen at some major or minor version number of Vector, though, so that it's easier for users to understand the dependencies.

[Saransh-cpp]

I think it would be easiest to support only one version.

Noted! I will create a PR to get rid of v1's support once awkward v2 (no rc) is out.

The switchover from requiring awkward>=1 to awkward>=2 should happen at some major or minor version number of Vector, though, so that it's easier for users to understand the dependencies.

This is a bit tricky 😅 We have the 2D object type constructors merged in, but the 3D and 4D ones are still under review (#231, #232). The awkward v2 support requires a release, but it might be weird if the release comes out with 2D constructors and not 3D+4D.

Should we do a 0.11.0 with 2D constructors and awkward>=2 support?

[Saransh-cpp]

Ah, now I remember why I was using v0.11.0 everywhere. We have new features and bug fixes in the main branch right now, and tagging a new version would include those changes as well (unless there is some other way?).

I will stick to v0.11.0 for now.

Changelog for v0.11.0 - https://vector.readthedocs.io/en/latest/changelog.html#version-0-11

[Saransh-cpp]

A gentle review request (again 😬): @jpivarski @henryiii

[jpivarski]

Sorry that I was slow to respond: this is an important fix!

It looks like this PR makes Vector work for both Awkward v1 and v2. There's this line, asking explicitly for v2:

vector/environment.yml

Lines 13 to 14 in d2a83af

	- "awkward>=2.0.0"
	- "uproot>=5.0.0"

but I think that's just for notebook examples. The pyproject.toml is only asking for Awkward >= 1.2 (and then only as an opt-in dependency):

vector/pyproject.toml

Lines 49 to 51 in d2a83af

	awkward = [
	"awkward>=1.2",
	]

(and similar for dev, etc.)

I see that you're even testing both versions of Awkward:

https://github.com/Saransh-cpp/vector/blob/d2a83af072f9504216dc92280c79d91a77bd5e7b/.github/workflows/ci.yml#L86-L87

and

https://github.com/Saransh-cpp/vector/blob/d2a83af072f9504216dc92280c79d91a77bd5e7b/.github/workflows/ci.yml#L118-L119

so that's all good.

I think this should be merged! It's satisfying all the requirements, and (if I remember right) there were some people asking for it.

(I'm also reminded that I need to update the disassemble check.)

[jpivarski]

Now this PR also fixes #294. I was originally thinking that I would update uncompyle6 to the latest Python version that it supports and edit the tests for any changes in bytecode between Python 3.8 and now (I expect very few because our compute functions are pure expressions, no splitting of code paths).

The question I had was when "now" is: Python 3.10? Python 3.11? But no! uncompyle6 still only supports Python 3.8! In fact, the latest it supports is Python 3.8.13, not even 3.8.15. (I'm surprised that patch releases matter—I would have thought that Python byte codes can't change from one patch release to the next.)

The test was failing because it was pinned to Python 3.8.8 and GitHub Actions no longer serves that Python release. I just changed the number to 3.8.13, and now the test is running again.

I hope that uncompyle6 manages to keep up with the new Python versions. Otherwise, I don't know what will happen on October 14, 2024 and Python 3.8 gets end-of-lifed. We might have to drop this test, which would be unfortunate because it's a guard against introducing compute functions that won't be able to support certain backends (e.g. JAX).

[Saransh-cpp]

Thanks for the review and the fix, @jpivarski! I hope uncompyle6 bumps up their compatibility in the future.

One last question before the holidays: should vector get a 0.11.0 release or a v0.10.1 release? @henryiii pointed out above that 0.(x+1) is a major version in ZeroVer and 0.0.(x+1) is a minor version.

[henryiii]

Numbers aren't expensive, and I don't think we have many people limiting Vector, so I think 0.11 is fine (though if there are no new features, and v1 is still supported, then 0.10.1 is fine too).

[Saransh-cpp]

Thanks! Merging this in and tagging 0.11.0!