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

Improve composability of @classmethod and @st.composite #2578

Closed
MaxG87 opened this issue Sep 1, 2020 · 5 comments · Fixed by #2634
Closed

Improve composability of @classmethod and @st.composite #2578

MaxG87 opened this issue Sep 1, 2020 · 5 comments · Fixed by #2634
Labels
legibility make errors helpful and Hypothesis grokable

Comments

@MaxG87
Copy link

MaxG87 commented Sep 1, 2020

I frequently use the following pattern:

@dataclass
class FooBarTestCase:
    foo: T1
    bar: T2

    @classmethod
    @st.composite
    def sample_instance(draw, cls):
        ...

@given(FooBarTestCase.sample_instance())
def test_foo_bar(foo_bar_tc):
    ...

Please note that the order of @classmethod and @st.composite as well as the corresponding function arguments must be as above, i.e. of swapped order.

Unfortunately, now mypy deduces types to draw: Type and cls: Callable[[st.SearchStrategy[...]], ...]. If I use the arguments, I get type checking warnings. The only ways I know of to get around false positives is:

  1. to provide type hints (quite clumsy and has to be replicated in every project)
  2. suppress warnings by # type: ignore (might suppress useful warnings later on)
  3. use @staticmethod (forces me to duplicate the class name)

I would be glad if there could be a better way to combine @classmethod and @st.composite. It would help if there could be a predefined type hint for draw I could use. For sure there might be even better ways I am not aware of.

@Zac-HD Zac-HD added the legibility make errors helpful and Hypothesis grokable label Sep 1, 2020
@Zac-HD
Copy link
Member

Zac-HD commented Sep 1, 2020

Unfortunately we can't do much about this at the moment, but I've been watching PEP 612 and the preceeding discussions for several years now!

Once Mypy supports the PEP, we'll be able to express the way @st.composite removes the first argument (at least under Python >= 3.10, or with the typing_extensions package). Until then, I'm afraid a # type: ignore comment is your best bet 😕

@Zac-HD
Copy link
Member

Zac-HD commented Sep 1, 2020

As a workaround specifically for @classmethod, we could probably work out some introspection to support

@st.composite
@classmethod
def f(cls, draw, ...):

Update: no, unfortunately we can't do anything about the order-of-arguments issue.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 21, 2020

@MaxG87 - why not use

@dataclass
class FooBarTestCase:
    foo: T1
    bar: T2

@given(st.builds(FooBarTestCase))  # or st.from_type(FooBarTestCase)
def test_foo_bar(foo_bar_tc):
    ...

FWIW the type of draw is Callable[[st.SearchStrategy[T]], T]. Unfortunately @st.composite cannot be properly type-annotated without PEP 612 (due in Python 3.10), but after that Mypy should 'just work'.

@MaxG87
Copy link
Author

MaxG87 commented Oct 21, 2020

I use the sample_instance(…) pattern when a simple st.builds does not suffice. Most of the time I introduce TestCase classes for one of two reasons:

  • there are interdependencies in the generated data, e.g. a sampled range of permissive values (think: prices) and a list of elements that needs to be in the range.
  • providing expected values in complicated tests (e.g.: test case would be an entire HTML construct, reference is some value that needs to be extracted from the correct location)

In neither of these cases st.builds(…) can be used. A standalone sampling function however would lead to needlessly complicated function names, e.g foo_bar_test_cases(…). Having FooBarTestCase.sample_instance(…) seems to be preferable to me.

However, I totally understand your reasons not to spoil the design of st.composite(…) for my very particular use case. I tried out using @staticmethod in the last few ways and its almost as good as using @classmethod. The drawbacks are so minor that they hardly count.

So, thank you for discussing this issue. I'll stick with @staticmethod for now.

@Zac-HD
Copy link
Member

Zac-HD commented Oct 21, 2020

Makes sense! If you don't need to pass arguments (aside from the class) to your strategy, using st.register_type_strategy(FooBarTestCase, FooBarTestCase.sample_instance(FooBarTestCase)) - or a standalone function - and then st.from_type(FooBarTestCase) might also be a nice pattern.

To be clear, I do intend to support this as best as we can, but as per #2634 that's just not going to be great before Python 3.10 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legibility make errors helpful and Hypothesis grokable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants