[TYP] return array dtype is always unkown but it's actually specified as parameter

[FrancescElies]

Given the following example

import numpy as np
from hypothesis.extra.numpy import arrays

a = arrays(np.int32, shape=2)
reveal_type(a)

On master running the typechecker the ndarray dtype is unkown

# mytest.py:5: note: Revealed type is "hypothesis.strategies._internal.strategies.SearchStrategy[numpy.ndarray[Any, Any]]"

If my understanding is correct, after this pr the dtype parameter will be taken into account.

# mytest.py:5: note: Revealed type is "hypothesis.strategies._internal.strategies.SearchStrategy[numpy.ndarray[Any, numpy.dtype[def (Union[typing.SupportsInt, builtins.str, builtins.bytes, typing.SupportsIndex] =) -> numpy.signedinteger[numpy._typing._32Bit]]]]"

[Zac-HD]

Thanks for the PR! I'm looking forward to getting this in, though (per comments below) it might be a bit fiddly.

See also CONTRIBUTING.rst, for how to create a RELEASE.rst file with changelog (RELEASE_TYPE: patch) and add your name to the list of authors 😁

[Zac-HD]

I think this might increase our minimum supported numpy version. That's fine if we have to, but I think we could probably use an if TYPE_CHECKING: block and string annotations to preserve compatibility?

[FrancescElies]

Right, I only checked for py37 compatibility.

NDArray was introduced in numpy 1.21 and DTypeLike in v1.20, v1.21 is from june 2022, I belive that's old enough,
I would personally introduce a install_requires=['numpy>=1.21'] in the setup.py, but the call is totally yours.

What do you think?

[Zac-HD]

That's as old as NEP-29 suggests supporting, which seems a quite principled way to make the decision. @honno do you have thoughts on this?

Regardless we should set our actual minimum version in extras_require - I doubt we actually still support back to 1.9.0 - and think about setting up a minimum-numpy-version CI job. The CI job should be a separate PR though!

[FrancescElies]

That's as old as NEP-29 suggests supporting, which seems a quite principled way to make the decision. @honno do you have thoughts on this?

I missed that, then we are good to go, no need for if TYPE_CHECKING right?

[Zac-HD]

We could also declare the shape here, for constant shapes... albeit by going from two to four overloads (value or strategy for each of two arguments).

[FrancescElies]

I think we can get away without overloads in this case, by defining another typevar for shape but honestly I would like to leave this point for now until we sort out the rest the comments you made in this PR.

[Zac-HD]

Sounds good to me, I'll be very happy to get in whatever improvement we have and write up an issue with the rest. 👍

[Zac-HD]

Thanks again Francesc! I'm sorry it took a while for me to get back to this, but delighted to be improving support for type-inference in our API 🙂

[FrancescElies]

@Zac-HD sorry for the late reply, I was out of town.

No worries I also left this pr on the side and didn't realize how close we were to get that CI green, thanks for the lint fixes!

Until the next time 👋🙂