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

st.builds() works incorrectly with annotated __init__ #2603

Closed
sobolevn opened this issue Sep 6, 2020 · 3 comments · Fixed by #2606
Closed

st.builds() works incorrectly with annotated __init__ #2603

sobolevn opened this issue Sep 6, 2020 · 3 comments · Fixed by #2606
Labels
bug something is clearly wrong here

Comments

@sobolevn
Copy link
Member

sobolevn commented Sep 6, 2020

The simplest reproduction is:

from typing import Generic, TypeVar

from hypothesis import given
from hypothesis import strategies as st

_ValueType = TypeVar('_ValueType')


class Wrapper(Generic[_ValueType]):
    _inner_value: _ValueType

    def __init__(self, inner_value: _ValueType) -> None:
        self._inner_value = inner_value


@given(st.builds(Wrapper))
def test_wrapper(wrapper):
    assert isinstance(wrapper, Wrapper)

Outputs:

====================================== FAILURES ======================================
____________________________________ test_wrapper ____________________________________

    @given(st.builds(Wrapper))
>   def test_wrapper(wrapper):

ex2.py:24: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.7/site-packages/hypothesis/core.py:684: in _execute_once_for_engine
    escalate_hypothesis_internal_error()
.venv/lib/python3.7/site-packages/hypothesis/core.py:655: in _execute_once_for_engine
    result = self.execute_once(data)
.venv/lib/python3.7/site-packages/hypothesis/core.py:610: in execute_once
    result = self.test_runner(data, run)
.venv/lib/python3.7/site-packages/hypothesis/executors.py:52: in default_new_style_executor
    return function(data)
.venv/lib/python3.7/site-packages/hypothesis/core.py:550: in run
    args, kwargs = data.draw(self.search_strategy)
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:889: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/collections.py:57: in do_draw
    return tuple(data.draw(e) for e in self.element_strategies)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/collections.py:57: in <genexpr>
    return tuple(data.draw(e) for e in self.element_strategies)
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/strategies.py:650: in do_draw
    result = self.pack(data.draw(self.mapped_strategy))
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/lazy.py:150: in do_draw
    return data.draw(self.wrapped_strategy)
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/strategies.py:650: in do_draw
    result = self.pack(data.draw(self.mapped_strategy))
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/collections.py:57: in do_draw
    return tuple(data.draw(e) for e in self.element_strategies)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/collections.py:57: in <genexpr>
    return tuple(data.draw(e) for e in self.element_strategies)
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/lazy.py:150: in do_draw
    return data.draw(self.wrapped_strategy)
.venv/lib/python3.7/site-packages/hypothesis/internal/conjecture/data.py:884: in draw
    return strategy.do_draw(self)
.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/strategies.py:650: in do_draw
    result = self.pack(data.draw(self.mapped_strategy))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

value = ((), {})

>       lambda value: target(*value[0], **value[1])  # type: ignore
    )
E   TypeError: __init__() missing 1 required positional argument: 'inner_value'

.venv/lib/python3.7/site-packages/hypothesis/strategies/_internal/core.py:1313: TypeError

Why does this happen?

My debugging session shows that:

  • required here is equal to {'inner_value'}
  • But, since here we fetch annotations from the object itself, not its __init__ method. It results in {'_inner_value': ~_ValueType}
  • And since these two sets do not match, the resulting set here is empty
  • And that's why we have this error 😢

I can fix it! The only question I have is:
Is it ok to chage how we get annotation from target if we are dealing with __init__?

@sobolevn
Copy link
Member Author

sobolevn commented Sep 6, 2020

Ok, this happens because of this line: https://github.com/HypothesisWorks/hypothesis/blob/hypothesis-python-5.31.0/hypothesis-python/src/hypothesis/internal/compat.py#L157-L158 Introduced in 5.31.0

At this point hints = {'_inner_value': ~_ValueType} and __init__ is not checked.

Related #2388

I am not sure how to fix this 🤔

@sobolevn
Copy link
Member Author

sobolevn commented Sep 6, 2020

Actually, no. It was working the same way even before 5.31.0.

        try:
            hints = typing.get_type_hints(thing)
        except (TypeError, NameError):
            hints = {}
        if hints or not inspect.isclass(thing):
            return hints
        try:
            return typing.get_type_hints(thing.__init__)
        except (TypeError, NameError, AttributeError):
            return {}

Here, if hints always evaluates to true for classes that have at least one annotation available. Mine has.
So, __init__ is never checked.

Proposed solution:

  • during the st.builds we can tell which mode we are using right now: object, object with __init__ or function, method
  • this way we can get annotations differently for different types

So, I can rewrite some parts of st.builds to handle this correctly. With backward compatibility.

@Zac-HD
Copy link
Member

Zac-HD commented Sep 7, 2020

An alternative would be to try each method of getting hints in order of preference (typing.get_type_hints(), __signature__, __init__) and keep the additional hints from each. This conceptually kinda ugly, but at least keeps all the problems in one place!

Happy to review whatever approach you think will work though :-)

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