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

Use ParamSpec to precisely annotate @st.composite and st.functions() #3396

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Jul 4, 2022

Being a follow-up to #3219, now that Mypy support for Concatenate has been out for long enough for most users to pick up - since 0.950, April 2022.

(We explictly don't promise any degree of compatibility with anything but a best-effort attempt to support the latest state of the static-type-checking ecosystem, but giving people more than a few days to update like this seems reasonable to me. The whole point of this is to provide a better user experience, after all!)

@Zac-HD Zac-HD requested a review from sobolevn July 4, 2022 02:30
@Zac-HD Zac-HD force-pushed the paramspec-for-composite branch from 61fecab to d409c56 Compare July 4, 2022 02:55
@Zac-HD Zac-HD requested a review from DRMacIver as a code owner July 4, 2022 02:55
@Zac-HD Zac-HD force-pushed the paramspec-for-composite branch from d409c56 to d691b6f Compare July 4, 2022 03:06
@Zac-HD Zac-HD force-pushed the paramspec-for-composite branch from d691b6f to 7bfb02e Compare July 4, 2022 03:56
@@ -1001,7 +1001,7 @@ def as_param(name, kind, defaults):
def given(
*_given_arguments: Union[SearchStrategy[Any], InferType],
) -> Callable[
[Callable[..., Union[None, Coroutine[Any, Any, None]]]], Callable[..., None]
[Callable[..., Optional[Coroutine[Any, Any, None]]]], Callable[..., None]
Copy link
Member

Choose a reason for hiding this comment

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

I think that it is safe to say that we cannot currently annotate this with ParamSpec, because of how complex its argument addition is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alas, yes, it's a partial-bind of only positional-or-keyword xor keyword-only parameters; and you can't pass strategies as positional args if there are any pos-only parameters; and positional args are bound from the rightmost pos-or-kw parameter (bizzare semantics, but we're stuck with it now).

I don't even see a more-precise overload we could extract, the closest would be "if you pass all the kwargs the wrapped function accepts, then the new function has no parameters", but even that breaks with **kwargs.

@@ -61,7 +61,7 @@ class ConstructivePredicate(NamedTuple):
predicate: Optional[Predicate]

@classmethod
def unchanged(cls, predicate):
def unchanged(cls, predicate: Predicate) -> "ConstructivePredicate":
Copy link
Member

Choose a reason for hiding this comment

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

If some class will subclass ConstructivePredicate without changing def unchanged, this annotation will become incorrect.

We can use TypeVar(..., bound="ConstructivePredicate") if this is the case.

Copy link
Member Author

@Zac-HD Zac-HD Jul 4, 2022

Choose a reason for hiding this comment

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

This is a private/internal-only class and so should never be subclassed; I'd decorate it with @final if we'd dropped Py37 but it doesn't seem worth the compatibility shims in the meantime.

returns: Union[SearchStrategy[Any], InferType] = infer,
pure: bool = False,
) -> SearchStrategy[Callable[..., Any]]:
"""functions(*, like=lambda: None, returns=..., pure=False)
Copy link
Member

Choose a reason for hiding this comment

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

Can we reuse __doc__ somehow? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately I couldn't see a way to do that which worked for docs-on-hover in IDEs; that's usually based on purely static information 😢

@Zac-HD Zac-HD force-pushed the paramspec-for-composite branch from 7bfb02e to 860bb3a Compare July 4, 2022 22:18
@Zac-HD Zac-HD merged commit 45b35c5 into HypothesisWorks:master Jul 4, 2022
@Zac-HD Zac-HD deleted the paramspec-for-composite branch July 4, 2022 22:49
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.

2 participants