reduce runtime dependency from setuptools to just packaging

[veprbl]

[codecov[bot]]

Codecov Report

Merging #1562 (8616456) into main (e692946) will decrease coverage by 0.00%. The diff coverage is 14.28%.

:exclamation: Current head 8616456 differs from pull request most recent head 524311a. Consider uploading reports for the commit 524311a to get more accurate results

Additional details and impacted files

Impacted Files	Coverage Δ
src/awkward/_v2/config.py	0.00% <0.00%> (ø)
src/awkward/_v2/_util.py	83.79% <100.00%> (ø)

[jpivarski]

FYI, @henryiii had a reason for Awkward to depend on setuptools, and the explicit use of it to get package versions was on top of that. I don't know what that reason was. I'll be getting back to this soon...

[henryiii]

For pkg_resources (should be swapped out for importlib_metadata).

[veprbl]

importlib_resources suggests to do a bit of a legwork to keep the file alive: https://importlib-resources.readthedocs.io/en/latest/migration.html

[jpivarski]

I removed references to pkg_resources as described in the migration guide.

@henryiii, if that's everything, if this can be merged as-is, please press the "squash-and-merge" button. (I don't personally know that there aren't any hidden dependencies not being specified here. This is the reason I requested a review from you, to make it a to-do item. Thanks!)

[henryiii]

importlib_resources needs to be added to requirement-test.txt. I'd normally add importlib_resources; python_version<"3.8" or similar and make the import toggle between importlib_resources and importlib.resources, but this is just in tests so I think it's fine.

[henryiii]

Oh, is it in src too? I checked for importlib_metadata by mistake, which is only in tests. So yes, test dep on importlib_metadata, and runtime dep on importlib_resources for some minimum Python version, I think 3.9 for files, and imports need to toggle between importlib_resources and importlib.resources based on Python version.

[jpivarski]

In 6d1a3a42a47a7937878f947bb8309543d04d9e9e, I added importlib_resources as a strict dependency if Python < 3.7 (the documentation on importlib.resources says "New in version 3.7") and imported from the standard library if possible, falling back to importlib_resources if necessary.

As I understood from some other conversation (email to Scikit-HEP admins?), we'll be dropping Python 3.6 support soon. That would make some of the previous commit unnecessary, but I think there's a Python version updater in pre-commit that would fix it.

[jpivarski]

I don't understand why these tests are failing. I can run them locally (Python 3.9).

Hmm.

AttributeError: module 'importlib.resources' has no attribute 'files'

Aha! This was added in Python 3.9: https://docs.python.org/3/library/importlib.html#importlib.resources.files

And that's the recommended translation of the pkg_resources method. So is it the case that we can't do this migration (merge this PR) until our minimum Python version is 3.9? That will be a few years.

[henryiii]

… runtime dep on importlib_resources for some minimum Python version, I think 3.9 for files, and imports need to toggle between importlib_resources and importlib.resources based on Python version.

[agoose77]

I suggested that we hold off on this for the final RC, simply because I wanted to think about the ramifications of loading kernels via the importlib.resources mechanism. Particularly because zipapps would likely not work. Of course, this actually isn't a problem for us - you just shouldn't do that, and I can't think of any reason someone would; zipapps need to be portable, and Awkward Array isn't (as in, we have compiled components).

[henryiii]

Particularly because zipapps would likely not work.

What? I thought the point of importlib.resources is that all loaders (including zip) are treated equally? Setuptools is not supposed to be a runtime dependency of anything, and we are working on removing it from being installed in all virtual environments as it is now.

[agoose77]

I thought the point of importlib.resources is that all loaders (including zip) are treated equally?

Right - to be clear, this PR wouldn't break anything that is already not-working. I wrote that comment after speaking to Jim in a meeting and suggesting we hold off for a second whilst I looked at it in more detail. Explicitly, I wanted to think about whether we could be doing anything better w.r.t library loading to be well behaved.

My assertion about zipapps is that the _ext module won't be loadable from a zipapp by default. I was also concerned about loading our shared-objects explicitly, but that's not actually an issue as they don't link to one-another, so extracting a single object would be fine.

This is not a stopper though - I don't think we need to support zipapps. I was just thinking "if we need to change this, let's try and keep all our options open" as a first reflex.

[henryiii]

My assertion about zipapps is that the _ext module won't be loadable from a zipapp by default

The context manager turns the virtual file into a temporary real one, so it should be. Though it might not work on Windows on Python 3.8+ due to the protection against loading DLLs from arbitrary directories, so you might need to manually add it.

Not sure ZipApps were ever intended to work with binary packages, though. You'd need one per target or a really big one with all arches and OS's.

[agoose77]

The context manager turns the virtual file into a temporary real one, so it should be.

Ah, I'm referring to the other code we have that imports _ext. These dll imports don't work for zipapps.

Not sure ZipApps were ever intended to work with binary packages, though. You'd need one per target or a really big one with all arches and OS's.

No, indeed, it's an academic point that has no bearing on us shipping this PR :laughing:

[jpivarski]

We discussed this in our Awkward/Uproot meeting today (@henryiii was there, @veprbl was not), and concluded that it's important to get this in before splitting git branches into main and main-v1 (see #1626), and that it is ready. The dependency on importlib_resources if Python < 3.9 is what makes that work.

So when the above tests pass, I'll merge it. Actually, I'll enable auto-merge now.