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

ENH: Add entry point for Array API implementation #19800

Merged
merged 6 commits into from
Nov 4, 2021

Conversation

honno
Copy link
Contributor

@honno honno commented Aug 31, 2021

On @mattip's suggestion, this PR simple adds an entry point for NumPy's Array API implementation in setup.py:

        entry_points={
            'console_scripts': f2py_cmds,
+           'array_api': ['numpy = numpy.array_api'],
        },

The Data APIs consortium is currently discussing whether Array API adaptors should have an entry point to point to their actual implementation in data-apis/array-api#244. In summary: for array consuming libraries you can currently get the Array API namespace via xp = x.__array_namespace__(), but for array generating libraries (e.g. Hypothesis in HypothesisWorks/hypothesis#3065) the entry point would allow programmatic discovery of the available Array API implementations in the Python environment.

>>> from importlib.metadata import entry_points
>>> eps = entry_points()
>>> eps["array_api"]
(EntryPoint(name='numpy', value='numpy.array_api', group='array_api'), ...)
>>> ep = next(ep for ep in eps["array_api"] if ep.name == "numpy")
>>> xp = ep.load()

I thought I'd make this PR more-so to demonstrate how the proposal would work in practice, and it's something that can be returned to if the proposal goes through.

@mattip
Copy link
Member

mattip commented Aug 31, 2021

Could you add a test to ./numpy/tests/test_public_api.py for the entry points?

@honno
Copy link
Contributor Author

honno commented Aug 31, 2021

Could you add a test to ./numpy/tests/test_public_api.py for the entry points?

Hope something like this is okay. Note my test uses importlib.metadata which is 3.8+ (also numpy.array_api)... I'm not sure how to "skip" 3.7 versions like I suppose you do with the Array API tests (I can explore tomorrow).

@mattip
Copy link
Member

mattip commented Aug 31, 2021

NumPy has dropped support for 3.7 in CI testing, we are now 3.8+ only (on main for 1.22, we may release another 1.21 bugfix that will still support 3.7 at some point)

@mattip
Copy link
Member

mattip commented Sep 1, 2021

assert "array_api" in eps.keys() is apparently not the correct interface to check on CPython 3.10. I think it should be shortened to assert "array_api" in eps instead. Also for some reason in the debug run this check fails.

@honno
Copy link
Contributor Author

honno commented Sep 1, 2021

So the issue is that <3.10 eps is just a dict, but in 3.10 it's something else (a SelectableGroup)—see these release notes. "array_api" in eps will indeed work for either case, but in 3.10+ you'll get a DeprecationNotice for not using the 3.10+ "select" interface e.g. eps.select(group="array_api"). There is a backport of the 3.10+ select interface in the backports.entry-points-selectable package. Otherwise it seems you'll need a specific tests for 3.8/3.9 and a specific test for 3.10+, or I can do something ugly and use try: ... except AttributeError: ... for each assertion.

@mattip
Copy link
Member

mattip commented Sep 1, 2021

It seems the select interface is 3.7+ compatible, so it should be fine to use the new interface in the test.

@honno
Copy link
Contributor Author

honno commented Sep 1, 2021

Not on my machine (the release notes say v3.7.0 but thats for importlib-metadata, not Python).

@Zac-HD
Copy link
Contributor

Zac-HD commented Sep 1, 2021

In Hypothesis we just try to use the new interface, and fall back to the old:

    try:
        eps = importlib_metadata.entry_points(group="array_api")
    except TypeError:
        # Load-time selection requires Python >= 3.10 or importlib_metadata >= 3.6,
        # so we'll retain this fallback logic for some time to come.  See also
        # https://importlib-metadata.readthedocs.io/en/latest/using.html
        eps = importlib_metadata.entry_points().get("array_api", [])

@honno
Copy link
Contributor Author

honno commented Sep 1, 2021

I followed Zac's example of fall-backing to the dict interface, so now (locally) the test runs nicely on both Python 3.10 and 3.8 🥳

@honno honno force-pushed the xp-entry-point branch 4 times, most recently from 6f54c21 to 79ff375 Compare September 1, 2021 10:28
@honno
Copy link
Contributor Author

honno commented Sep 1, 2021

3.10 now works in CI, just not the debug server. The failing assertion:

assert len(xp_eps) > 0

i.e. no Array API entry points could be found... so it's likely the debug server doesn't build NumPy in such a way it looks at the specified entry point?

It's 3.8, but the tests work locally on my 3.8.10 env. I can't figure out what USE_DEBUG changes that affects this test case. Mind I'm rather ignorant on NumPy's build process heh.

@asmeurer
Copy link
Member

asmeurer commented Sep 1, 2021

I would wait until we actually decide if we are going to do data-apis/array-api#244 and finalize it in the spec before merging this.

@asmeurer
Copy link
Member

asmeurer commented Sep 1, 2021

With that being said, it's useful to have an implementation. If anyone has any comments on this approach, and whether you like it or not, that would useful to hear as well.

@mattip
Copy link
Member

mattip commented Sep 1, 2021

This seems very straightforward and simple to implement. Entry points have proven useful for command-line tools. It does suffer from the "one more standard" problem, but I think it is indeed useful.

@charris charris changed the title Entry point for Array API implementation ENH: Add entry point for Array API implementation Sep 9, 2021
@mattip
Copy link
Member

mattip commented Oct 19, 2021

@asmeurer has the Array API group reached a conclusion about entry points?

@asmeurer
Copy link
Member

People at the consortium meeting were generally amenable to the idea, but nothing has been added to the spec about it yet.

@honno
Copy link
Contributor Author

honno commented Oct 26, 2021

Not sure what's up with Travis, but in regards to failing on debug job I have no idea. I burnt an hour trying to figure out what exactly in the build script (with the debug flag) doesn't expose the entrypoints... it's something I bet someone with experience with developing NumPy or packaging in general could solve quite easily heh.

@rgommers
Copy link
Member

rgommers commented Nov 1, 2021

That entrypoints don't work in a debug build seems quite odd, but possibly is a known issue: microsoft/debugpy#165 is quite similar. The code in setup.py here is very simple. Maybe an importlib bug?

A pragmatic approach would be to skip this test on a debug build, which can be checked like so:

import sysconfig
sysconfig.get_config_var('Py_DEBUG')

Then open an issue to follow up.

@mattip
Copy link
Member

mattip commented Nov 2, 2021

I could not reproduce this locally, this passes (on Ubuntu 20.04 focal, like the CI run):

python3-dbg -m venv /tmp/venv
source /tmp/venv/bin/activate
python runtests.py -t numpy/tests/test_public_api.py -vv

Skipping the test on debug sound like a good solution. I didn't realize how new the importlib module is. It changed the interface for entry points between 3.9 to 3.10, and there are other discussions that point to its immaturity, even though it is in the stdlib.

@honno honno force-pushed the xp-entry-point branch 2 times, most recently from 5cd1a87 to 3b0f5ce Compare November 2, 2021 18:05
@honno
Copy link
Contributor Author

honno commented Nov 4, 2021

I now xfail the test when sysconfig.get_config_var('Py_DEBUG') is not None. As @mattip suggests, it doesn't seem to be a problem with python-dbg builds specifically, so I clarify in the xfail reason that we're only inferring the debug job was run. Hope an xfail as opposed to a skip is okay—it seemed more appropriate and useful.

Optional entry points are now in the spec 🙂

@mattip
Copy link
Member

mattip commented Nov 4, 2021

Just a wild guess that this may have to do with using the system python3-dbg on that particular image. But I am happy to xfail the test.

@mattip mattip merged commit 8eb6555 into numpy:main Nov 4, 2021
@mattip
Copy link
Member

mattip commented Nov 4, 2021

Thanks @honno

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants