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

Provide SearchStrategy optionals #2765

Closed
MaxG87 opened this issue Jan 22, 2021 · 3 comments · Fixed by #2768
Closed

Provide SearchStrategy optionals #2765

MaxG87 opened this issue Jan 22, 2021 · 3 comments · Fixed by #2768
Labels
new-feature entirely novel capabilities or strategies

Comments

@MaxG87
Copy link

MaxG87 commented Jan 22, 2021

I noticed that I frequently write my own helper optionals:

_T = TypeVar("_T")
def optionals(strategy: st.SearchStrategy[_T]) -> st.SearchStrategy[Optional[_T]]:
    return st.one_of(st.none(), strategy)

Obviously, I use it to generate values for variables that either take a value or are None.

I wanted to ask if this coud be part of the strategies submodule. Then I could stop to copy that implementation from project to project. I expect it to be helpful for others too.

@rsokl rsokl added the new-feature entirely novel capabilities or strategies label Jan 22, 2021
@rsokl
Copy link
Contributor

rsokl commented Jan 22, 2021

Given that the shorthand

st.none() | strategy  # equivalent to `st.one_of(st.none(), strategy)`

is already available to express this, I wouldn't think adding optionals() to the API is warranted.

That being said, I do notice that st.none() | strategy doesn't have the desired type annotation associated with it - it gets resolved as SearchStrategy[None].

@Zac-HD do you think adding an overload for the case of a two-argument invocation of one_of would be useful in terms of providing more accurate type annotation resolution for the | syntax?

(IGNORE ALL OF THE BELOW - MY IDE WASNT CONFIGURED CORRECTLY)

Also, an aside...

As I was playing with @MaxG87 's optionals implementation, I noticed some weird behavior from pylance...

image

Any idea why the above signature might produce this confused annotation, @Zac-HD ? Perhaps something for us to point out to the pyright folks.

@MaxG87
Copy link
Author

MaxG87 commented Jan 22, 2021

The proposed shorthand is good for a single usage but cannot be reused. Having the optionals strategy would allow

@given(st.optionals(st.text()), st.optionals(st.integers()))
def test_can_handle_missing_info(name, age):

If I had to use the shorthand, I either would have to duplicate the logic or predefine things like maybe_text = st.none() | st.text() everywhere where needed. It also would not be very flexible. For example, I cannot control the length of maybe_text directly, let alone character strategies.

A very weak argument at last: The easiness of st.just(None) did not hinder the introduction of st.nones().

@Zac-HD
Copy link
Member

Zac-HD commented Jan 22, 2021

A very weak argument at last: The easiness of st.just(None) did not hinder the introduction of st.nones()

It also predates our API design guide, so. I'd still be inclined to accept it on the basis that None really is specially common, but there'd be an argument against now too.

Why the confused annotations?

Looks like SearchStrategy.__or__ is not annotated! I'll open a PR with type hints for that and st.one_of() 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature entirely novel capabilities or strategies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants