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

lists with unique_by → one_of → keyword-only composite = TypeError #1999

Closed
SydneyUni-Jim opened this issue Jun 2, 2019 · 3 comments · Fixed by #2000
Closed

lists with unique_by → one_of → keyword-only composite = TypeError #1999

SydneyUni-Jim opened this issue Jun 2, 2019 · 3 comments · Fixed by #2000
Labels
bug something is clearly wrong here

Comments

@SydneyUni-Jim
Copy link
Contributor

Hypothesis 4.24.0, pytest 4.6.0, Python 3.7.3, macOS 10.14.5

A TypeError is raised when a lists strategy with the unique_by argument is chained to a one_of strategy chained to a composite strategy that only has keyword-only arguments.

The following code demonstrates the issue. It raises TypeError: g1() got an unexpected keyword argument 'kwarg1'.

from hypothesis import given
import hypothesis.strategies as st

@st.composite
def g1(draw, *, kwarg1=None):
  return draw(
    st.fixed_dictionaries({'kwarg1': st.just(kwarg1), 'i': st.integers()})
  )

def g2(*, kwarg1=None):
  return st.one_of(g1(kwarg1=kwarg1))

def g3(*, kwarg1=None):
  return st.lists(
    g2(kwarg1=kwarg1),
    min_size=1,
    unique_by=lambda x: x['i']
  )

@given(g3())
def test_1(a):
   assert True

The TypeError is being raised by convert_keyword_arguments in reflection.py.

    if kwargs and not argspec.varkw:
        if len(kwargs) > 1:
            raise TypeError(
                "%s() got unexpected keyword arguments %s"
                % (function.__name__, ", ".join(map(repr, kwargs)))
            )

When the TypeError is raised, convert_keyword_arguments is being called (indirectly) by event_to_string in engine.py as it tries to str a lazyformat object that has a __format_string of 'Retried draw from %r to satisfy filter'.

After applying any one of the following patches to the demonstration code, the TypeError is no longer raised.

Don't use a keyword-only argument in the composite.

@@ -5 +5 @@
-def g1(draw, *, kwarg1=None):
+def g1(draw, kwarg1=None):

Don't use a composite at the end of the chain.

@@ -4,3 +4,2 @@
-@st.composite
-def g1(draw, *, kwarg1=None):
-  return draw(
+def g1(*, kwarg1=None):
+  return (

Don't use one_of in the middle of the chain.

@@ -11 +11 @@
-  return st.one_of(g1(kwarg1=kwarg1))
+  return g1(kwarg1=kwarg1)

Don't use unique_by in the lists stratergy.

@@ -17 +17 @@
-    unique_by=lambda x: x['i']
+    # unique_by=lambda x: x['i']
@Zac-HD Zac-HD added the bug something is clearly wrong here label Jun 2, 2019
@Zac-HD
Copy link
Member

Zac-HD commented Jun 2, 2019

Wow! Thanks for the fantastic report - I imagine it took a while to track down. It's also detailed enough that I narrowed it down to the following patch pretty quickly:

diff --git a/hypothesis-python/src/hypothesis/internal/reflection.py b/hypothesis-python/src/hypothesis/internal/reflection.py
index ef070ab..3a157a0 100644
--- a/hypothesis-python/src/hypothesis/internal/reflection.py
+++ b/hypothesis-python/src/hypothesis/internal/reflection.py
@@ -182,7 +182,7 @@ def convert_keyword_arguments(function, args, kwargs):
             else:
                 raise TypeError("No value provided for argument %r" % (arg_name))

-    if kwargs and not argspec.varkw:
+    if kwargs and not (argspec.varkw or argspec.kwonlyargs):
         if len(kwargs) > 1:
             raise TypeError(
                 "%s() got unexpected keyword arguments %s"

If this fixes it for you, would you like to open a PR on that basis?

@SydneyUni-Jim
Copy link
Contributor Author

That does fix my tests.

I'm new to both property-based testing and Hypothesis. So I wanted to make sure it wasn't something I was doing wrong. And I was happy to get off the quick sand for a bit.

I'd be to organise a PR. But this something you've solved, not me.

@Zac-HD
Copy link
Member

Zac-HD commented Jun 2, 2019

Awesome! All you'll need to do for a pr: apply the patch above, put the test case in a new file in tests/nocover/ (with a descriptive name), and add a new file hypothesis-python/RELEASE.rst with the following contents:

RELEASE_TYPE: patch 

This patch fixes :issue:`1999`, a spurious bug raised when a :func:`@st.composite <hypothesis.strategies.composite>` function was passed a keyword-only argument. 

Thanks to Jim Nicholls for his fantastic bug report. 

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.

2 participants