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

When st.from_type() falls back to st.builds(), try passing infer for arguments with default values #2708

Closed
lsorber opened this issue Dec 19, 2020 · 4 comments · Fixed by #2726
Labels
enhancement it's not broken, but we want it to be better

Comments

@lsorber
Copy link

lsorber commented Dec 19, 2020

For functions, builds uses a just(default_value) strategy for kwargs with a default_value, but it also allows you to use infer to instead use something to the effect of just(default_value) | from_type(type_annotation) [1].

For Pydantic models, Hypothesis inspects the __signature__ [2] and will also use the equivalent of a just(default_value) strategy for fields with a default_value (cf. example below). It would be nice if we could also choose to use an infer-like strategy in this scenario. What would be the recommended workaround for that?

Side question: why was the default strategy for kwargs/fields with default values chosen to be just(default_value) instead of just(default_value) | from_type(type_annotation)?

from typing import Optional
import hypothesis.strategies as st
from pydantic import BaseModel

class Foo(BaseModel):
    bar: int
    baz: Optional[str] = None

st.from_type(Foo).example()  # `baz` will always be `None`, even though you might expect to see some `str`s

[1] https://hypothesis.readthedocs.io/en/latest/details.html#inferred-strategies
[2] #2388

@Zac-HD Zac-HD added the enhancement it's not broken, but we want it to be better label Dec 20, 2020
@Zac-HD
Copy link
Member

Zac-HD commented Dec 20, 2020

Why was the default strategy for kwargs/fields with default values chosen to be just(default_value) instead of just(default_value) | from_type(type_annotation)?

I think this, and the difference between builds(Foo) and from_type(Foo), is actually the heart of the matter.

builds(Foo, ...) calls Foo with arguments drawn from the provided strategies, and we later added support for inferring strategies for missing required arguments - which was previously an error. There's not actually a just(default_value) involved here; we're simply not providing any argument for parameters which have a default value unless explicitly told to do so via baz=infer.

On the other hand, from_type(Foo) does not have any particular meaning beyond "somehow generates instances of Foo - as implemented, we try looking up registered strategies for the exact type, then subtypes, and finally use builds() if we couldn't find anything. The 'working as intended' workaround is to st.register_type_strategy(Foo, st.builds(Foo, baz=infer) before st.from_type(Foo).


However, I think it would make sense to have from_type(Foo) call builds(Foo, baz=infer) internally - i.e. pass infer for args with default values - because the current conservative behaviour of builds() is an implementation detail of from_type() rather than part of the contract.

We would need to be careful not to break backwards compatibility by raising errors on arg: UnresolvableType = ...; this is certainly possible with some more introspection and a call to try: inferred_strategy.validate(); except ....

@Zac-HD Zac-HD changed the title Feature request: option to infer fields with default values in Pydantic models When st.from_type() falls back to st.builds(), try passing infer for arguments with default values Dec 20, 2020
@mristin
Copy link
Contributor

mristin commented Dec 22, 2020

@Zac-HD This also has implications when registering types with contracts in Hypothesis. When you register a strategy for Foo, from_type works correctly, where builds(Foo) does not look up the registered strategy for the type.

Is that behavior intented?

@Zac-HD
Copy link
Member

Zac-HD commented Dec 22, 2020

Yes, the current behaviour is intended.

For any registered type, from_type() will return whatever you registered, while builds() will call the constructor - which is the intended behaviour. The idea for this issue is that if nothing is registered and from_type() delegates to builds(), it would be appropriate to infer strategies for all arguments - after all, if that's not what the user wants they can always use builds() directly and/or register a strategy!

@mristin
Copy link
Contributor

mristin commented Dec 22, 2020

@Zac-HD thanks for the clarification. I'll document that the types with contracts in constructor should always use from_type and never builds. As these classes with contracts are automatically registered, this should work as expected.

jmanuel1 added a commit to jmanuel1/concat that referenced this issue May 4, 2023
This version changed the way `from_type` works in a way that caused
stack overflows in the strategies for `IndividualType` and `TypeSequence`
(the `Type` hierarchy is very recursive). See
https://hypothesis.readthedocs.io/en/latest/changes.html#v5-46-0,
HypothesisWorks/hypothesis#2708 and
HypothesisWorks/hypothesis#2726 for what changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement it's not broken, but we want it to be better
Projects
None yet
3 participants