Skip to content

v2022-12 tag is broken #631

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

Closed
jakevdp opened this issue May 24, 2023 · 8 comments
Closed

v2022-12 tag is broken #631

jakevdp opened this issue May 24, 2023 · 8 comments
Assignees

Comments

@jakevdp
Copy link

jakevdp commented May 24, 2023

The v2022-12 tag is broken due to the bug fixed in #455, which as far as I can tell has not yet been made part of a release or tag.

The error can be seen here: https://github.com/google/jax/actions/runs/5073141741/jobs/9111776151?pr=16099

$ Run pytest --ci array-api-tests/array_api_tests/
ImportError while loading conftest '/home/runner/work/jax/jax/array-api-tests/conftest.py'.
array-api-tests/conftest.py:9: in <module>
    from array_api_tests import _array_module as xp
array-api-tests/array_api_tests/__init__.py:8: in <module>
    from ._array_module import mod as _xp
array-api-tests/array_api_tests/_array_module.py:4: in <module>
    from . import stubs
array-api-tests/array_api_tests/stubs.py:31: in <module>
    name_to_mod[name] = import_module(f"signatures.{name}")
E     File "/home/runner/work/jax/jax/array-api-tests/array-api/spec/API_specification/signatures/linalg.py", line 44[7](https://github.com/google/jax/actions/runs/5073141741/jobs/9111776151?pr=16099#step:6:8)
E       """
E       ^^^
E   SyntaxError: invalid escape sequence '\*'

This is a github action on a JAX PR implementing the array API: jax-ml/jax#16099

Let me know if you have suggestions for how to properly configure the github action in order to pull-in the fix. Thanks!

@asmeurer
Copy link
Member

This should be easy enough to fix (just make the docstrings raw).

Although somewhat related to this, I wonder if we should get rid of the tags here? We now have all versions of the spec in the main branch of this repo, exactly so that we can fix little issues like this.

I guess when this is fixed we'll also need to update the submodule in the test suite @honno

@jakevdp
Copy link
Author

jakevdp commented May 24, 2023

True it's easy enough to fix locally by manually editing the problematic files. I'm looking for suggestions of the best way to get this working in my github actions job (configured here: https://github.com/google/jax/blob/26fde03dccc595158f556e895d976b4aa3d41a75/.github/workflows/jax-array-api.yml)

jakevdp added a commit to jakevdp/jax that referenced this issue May 24, 2023
@asmeurer
Copy link
Member

There were still some bad escapes in main which I've fixed at #632.

For array-api-tests, I think actually @honno still needs to update it to run against the main of this repo which has all the spec versions instead of the tags. So for now you'll need to work around it (a straightforward way is to manually compile the signatures with python -m compileall so that they aren't compiled with the syntax warnings turned into errors).

I'm going to leave this issue open because as I noted, we should figure out if we actually want to keep the tags on this repo.

@jakevdp
Copy link
Author

jakevdp commented May 24, 2023

I'm using sed as a workaround for now: jax-ml/jax@c55b9e7

@kgryte
Copy link
Contributor

kgryte commented Jun 29, 2023

@honno Has this been resolved or does more work remain?

@honno
Copy link
Member

honno commented Jun 29, 2023

Has this been resolved or does more work remain?

Haven't touched this yet (specifically writing a note in the README on tags and what they do/don't mean), need to find half an hour to sit down and do it 🤦

jakevdp added a commit to jakevdp/jax that referenced this issue Nov 8, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 10, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 10, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 10, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 13, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 13, 2023
jakevdp added a commit to jakevdp/jax that referenced this issue Nov 14, 2023
@honno
Copy link
Member

honno commented Mar 18, 2024

The issue @jakevdp had should be resolved by data-apis/array-api-tests#213 now.

we should figure out if we actually want to keep the tags on this repo.

@asmeurer If this is still interesting, could we close this and open a new issue? For 2023 we still kept with tags, which IMO makes sense.

@asmeurer
Copy link
Member

I guess "tagging at release" is probably fine, but we should be very clear that those tags shouldn't really be used, and are only there for historical context. You should always use whatever is in main, because all versions of the standard are there and things like typo fixes are backported.

For the test suite, we should pin to a commit from main, but update it from time to time. Maybe we could look at something like dependabot to do this. Or we could just do it manually like we do now.

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

No branches or pull requests

4 participants