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

compatibility with numpy>=2.0 #3955

Merged
merged 36 commits into from
Apr 26, 2024
Merged

compatibility with numpy>=2.0 #3955

merged 36 commits into from
Apr 26, 2024

Conversation

keewis
Copy link
Contributor

@keewis keewis commented Apr 24, 2024

This also replaces a couple of top-level pytest.warns with ignores, which was the simplest way I found to silence the errors caused by the change in behavior of numpy>=2.

Still missing: a way to actually test this with a nightly build of numpy, but since I don't know your CI well enough I'll need help with that.

With numpy>=2, I'm getting a bunch of other failures locally, mostly concerning the removal of numpy.asfarray.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @keewis! Both the implementation and the test changes look good to me. The CI setup is a little tricky, but we can imitate the "oldest numpy" job:

  1. set up a tox environment, like below, with this install command:
    # This test job runs on the oldest version of CPython we support, against the minimum
    # version specified in our runtime dependencies. For now, that's the oldest version
    # with a wheel for non-EOL Python. In future we might deprecate faster per NEP-29.
    [testenv:py38-oldestnumpy]
    deps=
    -r../requirements/test.txt
    allowlist_externals =
    bash
    commands=
    bash -c "pip install --only-binary=:all: numpy==$(grep 'numpy>=' setup.py | grep -oE '[0-9.]+')"
    python -bb -X dev -m pytest tests/numpy/ -n auto
  2. re-export that for our tooling (no need for the py= argument in this case):
    standard_tox_task("py38-oldestnumpy", py="3.8")
  3. invoke it for GH actions:
    - check-py38-oldestnumpy

and then I think we're done 🙂

hypothesis-python/RELEASE.rst Outdated Show resolved Hide resolved
hypothesis-python/tests/array_api/test_arrays.py Outdated Show resolved Hide resolved
keewis and others added 3 commits April 24, 2024 22:41
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
@Zac-HD
Copy link
Member

Zac-HD commented Apr 24, 2024

woohoo, some Numpy-2-specific CI failures!

  • looks like we need to wrap this in a pytest.param("a", marks=pytest.mark.skipif(<is numpy >= 2>) because the "a" alias for "S" has been removed
  • update this fails_with decorator to include OverflowError on Numpy >= 2

@keewis
Copy link
Contributor Author

keewis commented Apr 24, 2024

I've seen a lot more than these locally, but maybe those are not running because of the environment? Edit: nope, that was because I copied the tests/numpy restriction from the oldest-numpy job

Most notably, some ghostwriter tests error at test collection time:

  File ".../hypothesis/hypothesis-python/tests/ghostwriter/test_expected_output.py", line 132, in <module>
    ("magic_gufunc", ghostwriter.magic(numpy.matmul)),
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1301, in magic
    make_(_make_ufunc_body, func, annotate=annotate)
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1202, in make_
    imp, body = how(*args, **kwargs, except_=except_, style=style)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1884, in _make_ufunc_body
    call=_write_call(func, *ascii_lowercase[: func.nin], except_=except_),
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 781, in _write_call
    args = ", ".join(
           ^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 784, in <genexpr>
    if p.kind is inspect.Parameter.POSITIONAL_ONLY
       ^^^^^^
AttributeError: 'NoneType' object has no attribute 'kind'

and (after commenting out the first one):

File ".../hypothesis/hypothesis-python/tests/ghostwriter/test_expected_output.py", line 191, in <module>
    ("addition_op_multimagic", ghostwriter.magic(add, operator.add, numpy.add)),
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1301, in magic
    make_(_make_ufunc_body, func, annotate=annotate)
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1202, in make_
    imp, body = how(*args, **kwargs, except_=except_, style=style)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 1884, in _make_ufunc_body
    call=_write_call(func, *ascii_lowercase[: func.nin], except_=except_),
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 781, in _write_call
    args = ", ".join(
           ^^^^^^^^^^
  File ".../hypothesis/hypothesis-python/src/hypothesis/extra/ghostwriter.py", line 784, in <genexpr>
    if p.kind is inspect.Parameter.POSITIONAL_ONLY
       ^^^^^^
AttributeError: 'NoneType' object has no attribute 'kind'

Also, there appears to be an issue with your formatting checks, it complains that ruff is not installed.

@keewis
Copy link
Contributor Author

keewis commented Apr 24, 2024

in the most recent commit I removed the limitation to just tests/numpy. This means it runs all tests, which is probably not intended. Instead, maybe we just need to enumerate all those directories that make use of numpy?

@Zac-HD
Copy link
Member

Zac-HD commented Apr 24, 2024

Most notably, some ghostwriter tests error at test collection time:

here, I guess we need if p is None or p.kind...

in the most recent commit I removed the limitation to just tests/numpy. This means it runs all tests, which is probably not intended. Instead, maybe we just need to enumerate all those directories that make use of numpy?

Ah, for that copy the "niche" tests - that covers everything with optional dependencies.

(and thanks for pursuing this!)

@keewis
Copy link
Contributor Author

keewis commented Apr 25, 2024

Ah, for that copy the "niche" tests - that covers everything with optional dependencies.

Not sure if that can work, in scripts/other-tests.sh the pinned version of numpy will be enforced, replacing the nightly wheel we just installed.

@keewis
Copy link
Contributor Author

keewis commented Apr 25, 2024

here, I guess we need if p is None or p.kind...

It appears the error only occurs on python>=3.11. My guess is that this is because inspect.signature stopped raising for certain functions built in C (but the parameters returned are *args, **kwargs so not really useful) – I can't find the entry in the python changelog, though. I can also reproduce this on versions other than numpy>=2.0, so I think this is unrelated to this PR. No idea why CI doesn't fail, though.

@keewis
Copy link
Contributor Author

keewis commented Apr 25, 2024

the final failure on numpy>=2.0 appears to be caused by numpy.ndarray gaining the __array_namespace__ protocol.

In the most recent commits I've hacked together a wrapping mechanism that allows removing arbitrary attributes on the array returned by any of the functions in the namespace (I don't know if that also works on properties, though), which can then be used to patch out the __array_namespace__. Not sure if that's the best way to resolve this, though.

Edit: not sure what to make of the windows 3.9 cover+rest CI failure... it says "stack overflow", but the traceback looks like a recursion, which is weird. To make sure it's not the issue, we could explicitly call object.__getattribute__ instead of super().__getattribute__, though.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 25, 2024

Edit: not sure what to make of the windows 3.9 cover+rest CI failure... it says "stack overflow", but the traceback looks like a recursion, which is weird. To make sure it's not the issue, we could explicitly call object.__getattribute__ instead of super().__getattribute__, though.

Hmm. Might as well try this - it's certainly suspicious that we hit this with mocks around.

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tiny nits, not necessary to merge

hypothesis-python/tests/numpy/test_from_dtype.py Outdated Show resolved Hide resolved
@keewis
Copy link
Contributor Author

keewis commented Apr 25, 2024

Hmm. Might as well try this - it's certainly suspicious that we hit this with mocks around.

Didn't help, see the most recent commit. So I don't know what's wrong here.

@keewis keewis changed the title explicitly cast finfo.smallest_normal to builtin python float compatibility with numpy>=2.0 Apr 25, 2024
keewis and others added 2 commits April 25, 2024 19:50
@keewis
Copy link
Contributor Author

keewis commented Apr 26, 2024

my last attempt, replacing __getattribute__ with __getattr__, appears to have worked. I have no clue why, though!

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @keewis! Delighted to see this all working 🙂

@Zac-HD Zac-HD merged commit eccba85 into HypothesisWorks:master Apr 26, 2024
50 of 54 checks passed
@keewis keewis deleted the numpy-2.0 branch April 26, 2024 21:54
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

Successfully merging this pull request may close these issues.

Test for compatibility with Numpy prerelease (2.0 is coming this month)
2 participants