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

Problem with decorated __init__ #3029

Closed
mristin opened this issue Jul 8, 2021 · 2 comments · Fixed by #3095
Closed

Problem with decorated __init__ #3029

mristin opened this issue Jul 8, 2021 · 2 comments · Fixed by #3095

Comments

@mristin
Copy link
Contributor

mristin commented Jul 8, 2021

Hi,
While investigating #3026, I noticed that required_args function uses inspect.getfullargspec:

def required_args(target, args=(), kwargs=()):
    ...
        spec = inspect.getfullargspec(
            getattr(target, "__init__", target)
            if inspect.isclass(target) and not hasattr(target, "__signature__")
            else target
        )
   ...

Unfortunately, inspect.getfullargspec will not inspect the wrapper chain according to inspect.getfullargspec's current implementation in the stdlib:

def getfullargspec(func):
        ....
        # Re: `follow_wrapper_chains=False`
        #
        # getfullargspec() historically ignored __wrapped__ attributes,
        # so we ensure that remains the case in 3.3+

        sig = _signature_from_callable(func,
                                       follow_wrapper_chains=False,
                                       skip_bound_arg=False,
                                       sigcls=Signature,
                                       eval_str=False)

This means that when you use required_args in _from_type, you will get different behavior depending on whether the constructor has been decorated (no required) or not (required reflects the actual arguments). Mind that decorator in many cases does not change the signature; moreover, this behavior is the same even if functools.update_wrapper is called (due to inspect.getfullargspec).

Here is the relevant snippet from _from_type:

        required = required_args(thing)
        if required and not (
            required.issubset(get_type_hints(thing))
            or attr.has(thing)
            or is_typed_named_tuple(thing)  # weird enough that we have a specific check
        ):
            raise ResolutionFailed(
                f"Could not resolve {thing!r} to a strategy; consider "
                "using register_type_strategy"
            )

In particular, this will cause hypothesis.strategies.builds to ignore the arguments of the decorated __init__ with a rather confusing error message for the user.

Please let me know how you'd like to proceed with this issue. I could write a small snippet so that you can reproduce the issue etc.

@Zac-HD
Copy link
Member

Zac-HD commented Jul 21, 2021

A reproducing example would be really helpful, thanks!

Seems related to #961, #2603, #2495 and the "use inspect.signature()" point in #2218. This is a more difficult problem than it first appears, because sometimes it's correct to ignore decorators; and in other cases you shouldn't ignore them. Our current approach is a messy compromise to support most of the things people have reported, and changing it may well break things that currently work (even if they shouldn't work).

@mristin
Copy link
Contributor Author

mristin commented Jul 21, 2021

Hi @Zac-HD ,
Here's a short script that reproduces the issue:

import functools

from hypothesis import given
import hypothesis.strategies as st


def some_decorator(func):
    def wrapper(*args, **kwargs):
        return func(*args, **kwargs)

    functools.update_wrapper(wrapper=wrapper, wrapped=func)

    return wrapper


class SomeClass:
    @some_decorator
    def __init__(self, x: int) -> None:
        self.x = x


def main() -> None:
    @given(something=st.builds(SomeClass))
    def test(something):
        print(something.x)

    test()
    # Raises:
    # Traceback (most recent call last):
    #   ...
    # TypeError: __init__() missing 1 required positional argument: 'x'


if __name__ == "__main__":
    main()

Please let me know if you need any more details etc.

(Edit: added traceback)

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 a pull request may close this issue.

2 participants