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

Using overloaded composite strategies causes HypothesisDeprecationWarning #3970

Closed
stinodego opened this issue May 4, 2024 · 2 comments · Fixed by #3971
Closed

Using overloaded composite strategies causes HypothesisDeprecationWarning #3970

stinodego opened this issue May 4, 2024 · 2 comments · Fixed by #3971
Labels
enhancement it's not broken, but we want it to be better question not sure it's a bug? questions welcome

Comments

@stinodego
Copy link

stinodego commented May 4, 2024

I have a composite strategy that is overloaded, e.g. it can return either of two types depending on an input parameter. The code below type checks correctly, but it raises a warning on execution:

from __future__ import annotations

from typing import Literal, overload

import hypothesis.strategies as st
from hypothesis import given

@st.composite
@overload
def stuff(draw: st.DrawFn, *, return_str: Literal[False] = ...) -> int: ...

@st.composite
@overload
def stuff(draw: st.DrawFn, *, return_str: Literal[True]) -> str: ...

@st.composite
def stuff(draw: st.DrawFn, *, return_str: bool = False) -> int | str:
    i = draw(st.integers())
    if return_str:
        return str(i)
    else:
        return i

@given(s=stuff())
def test_stuff(s: int | str) -> None:
    print(s)

Running pytest, I get:

repro.py:9: in <module>
    @st.composite
../.venv/lib/python3.12/site-packages/hypothesis/strategies/_internal/core.py:1871: in composite
    return _composite(f)
../.venv/lib/python3.12/site-packages/hypothesis/strategies/_internal/core.py:1820: in _composite
    note_deprecation(
../.venv/lib/python3.12/site-packages/hypothesis/_settings.py:723: in note_deprecation
    warnings.warn(HypothesisDeprecationWarning(message), stacklevel=2 + stacklevel)
E   hypothesis.errors.HypothesisDeprecationWarning: There is no reason to use @st.composite on a function which does not call the provided draw() function internally.

So it looks like the composite decorator on the overload part of a function causes a false positive on this deprecation warning. The actual function does use draw.

Perhaps there is another way to define overloaded strategies that avoids this issue? My current workaround is to ignore the warning on the overload definitions:

with warnings.catch_warnings():
    warnings.simplefilter("ignore", category=HypothesisDeprecationWarning)
	
	@st.composite
	@overload
	def stuff(draw: st.DrawFn, *, return_str: Literal[False] = ...) -> int: ...
	
	@st.composite
	@overload
	def stuff(draw: st.DrawFn, *, return_str: Literal[True]) -> str: ...

@st.composite
def stuff(draw: st.DrawFn, *, return_str: bool = False) -> int | str:
    i = draw(st.integers())
    if return_str:
        return str(i)
    else:
        return i
@Zac-HD Zac-HD added question not sure it's a bug? questions welcome enhancement it's not broken, but we want it to be better labels May 4, 2024
@Zac-HD
Copy link
Member

Zac-HD commented May 4, 2024

You know, I think we should just stop warning about @overloaded functions, it's not hard.

here's a workaround in the meantime

I'd suggest treating @st.composite as an implementation detail, and annotating the overloads as accepting the arguments except for draw, and returning a st.SearchStrategy instead of a value:

@overload
def stuff(*, return_str: Literal[False] = ...) -> st.SearchStrategy[int]: ...

@overload
def stuff(*, return_str: Literal[True] = ...) -> st.SearchStrategy[str]: ...

@st.composite
def stuff(draw: st.DrawFn, *, return_str: bool = False) -> int | str:
    i = draw(st.integers())
    if return_str:
        return str(i)
    else:
        return i

@stinodego
Copy link
Author

Thanks, that 'workaround' looks like the correct way to do it. I almost had it like that, except I forgot to update the return type to a SearchStrategy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement it's not broken, but we want it to be better question not sure it's a bug? questions welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants