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

@given decorator doesn't supply "self" argument when used inline #961

Closed
ShadowLNC opened this issue Nov 3, 2017 · 6 comments · Fixed by #3088
Closed

@given decorator doesn't supply "self" argument when used inline #961

ShadowLNC opened this issue Nov 3, 2017 · 6 comments · Fixed by #3088
Labels
bug something is clearly wrong here

Comments

@ShadowLNC
Copy link

ShadowLNC commented Nov 3, 2017

If I "decorate" a class's method inline, it doesn't supply the self argument when the resultant function is called. If I accept an *args argument instead (as with show), it is passed to the method, so I do not understand why it doesn't work for a specific argument instead of all positional arguments.

Please note that because this is raising exceptions without handling, you will of course have to run the remaining lines by copy-pasting into the interpreter.

(I'm aware this is not the intended use case for given. It's a reproducible example of inconsistent behaviour that I found, but I will not be using this code in my project. It may be related to #198 as that also involves the @given decorator.)

Python 3.5.2 (x86_64, Win10), Hypothesis 3.33.0 (installed to a virtual environment).

from hypothesis import given, strategies as strat

class MyTest:
    @given(dq=strat.booleans())
    def decorated(self, **kwargs):
        # No *args as we don't have any extra positional arguments
        print(kwargs)

    def bare(self, **kwargs):
        print(kwargs)

    def show(*args, **kwargs):
        print(args, kwargs)

instance = MyTest()

# Works, even if not given functools.wraps(instance.bare)
(lambda **kwargs: instance.bare(**kwargs))()
instance.decorated()

# "self" not supplied, calling this gives this error:
# TypeError: bare() missing 1 required positional argument: 'self'
broken = given(dq=strat.booleans())(instance.bare)
broken()
instance.decorated = broken
instance.decorated()  # Still broken.

# With no argument, you can see it's still supplied, but not as "self".
given(dq=strat.booleans())(instance.show)()

# 2 Arguments supplied
given(dq=strat.booleans())(instance.bare)(instance)
@Zac-HD Zac-HD added the bug something is clearly wrong here label Nov 4, 2017
@Zac-HD
Copy link
Member

Zac-HD commented Nov 4, 2017

Hmm, this is pretty strange!

I think we might be able to do something with inspect.ismethod and inspect.isclass (which I did for builds() in Hypothesis 3.33.1), but it's going to be pretty complicated even to work out where the change should go.

For now: thanks for reporting it; I'll mark this as a bug but don't plan to work on it any time soon 😄

@DRMacIver
Copy link
Member

Yeah, I think this is the same issue as with builds - given has to do a bunch of really complicated argspec juggling, and when getting the argspec of a bound method you get the one for the unbound function, so it doesn't notice that self has been bound.

I'm in a similar boat to @Zac-HD. It's definitely a bug, it's definitely weird, but it has a pretty high pain to motivation ratio for working on it. :-) What's your actual use case that lead to you finding this?

@ShadowLNC
Copy link
Author

ShadowLNC commented Nov 20, 2017

The code I wrote was pretty terrible code, and it's since been deleted (I had an unsaved copy and then Atom crashed/updated and dropped unsaved changes). It was something like this:

for i in range(len(someiterable)):
    for combo in itertools.combinations(someiterable, n+1):
        functools.reduce(
            lambda fn, gv: given(**{gv[0]: gv[1]})(fn),
            combo, self.original_fn)()  # Call it too

The general premise being to generate multiple strategy sets, one per combination from a larger set of strategies.

I now know this "double-wraps" the function and is a bad idea, so I rewrote it to do something like this:

combos = [combo
          for n in range(len(somedict))
          for combo in itertools.combinations(somedict.items(), n+1)]

return strat.sampled_from(combos).map(dict).flatmap(strat.fixed_dictionaries)

(That's within a function which is passed to the @given decorator directly, instead.)

TLDR: I don't have a valid use case, so low priority is fine by me.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 20, 2017

Cool, low priority bug it is!

As a side note, itertools.combinations makes me wince. Assuming that somedict isn't tiny, you'll probably get better examples if you avoid generating combinations up front - which you'll need to do for sampled_from to work, even though combinations is lazy.

st.sets(st.sampled_from(sorted(somedict.items()))).map(dict).flatmap(st.fixed_dictionaries)

That is: construct a sorted list of items (so order, and therefore sampling, is reproducible across runs), take a subset, turn it back into a dict, and construct a strategy with fixed_dictionaries.

@Zac-HD
Copy link
Member

Zac-HD commented Mar 2, 2018

For future reference, the place to fix this is probably process_arguments_to_given in core.py - check if the test is a method; if so get the class it's bound to and use that as selfy. Not entirely clear to me whether this would work when the current code doesn't, but it's worth trying.

@cefisher1
Copy link

I saw that the other issues that I was working on have been fixed already (#2164) So I'm going to take a crack at this one. I'm going to look at process_arguements_to_given and see if I can find the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is clearly wrong here
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants