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

Pass **kwargs through npst.from_dtype() to the inferred strategy #2552

Closed
2 tasks
Zac-HD opened this issue Aug 19, 2020 · 1 comment · Fixed by #2619
Closed
2 tasks

Pass **kwargs through npst.from_dtype() to the inferred strategy #2552

Zac-HD opened this issue Aug 19, 2020 · 1 comment · Fixed by #2619
Assignees
Labels
enhancement it's not broken, but we want it to be better

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Aug 19, 2020

Consider the following pattern:

arrays(floating_dtypes(), shape, elements=floats(-10, 10, width=...))

If width=None, when the generated dtype is float32 we will trigger the warning for inexact representation of element values (which is useful in general, if annoying here). Alternatively, if width=32 then we have massively reduced the precision of the element values that we can generate when the dtype is float64. There are of course workarounds with e.g. flatmap in current use, but they're pretty ugly and awkward for new users.

I therefore propose that:

  • npst.from_dtype() should take **kwargs, and pass them through to whichever strategy function it chooses. In practice this would usually just be min_value and max_value for integers or floats, but using **kwargs gives a cleaner implementation and allows use with timestamps, complex numbers, string length, and so on.
  • The arrays() strategy would then take elements: Union[SearchStrategy, Mapping[str, Any]] = None. The mapping case would then be used as kwargs for a call to from_dtype().

CC co-designer @rsokl.

@Zac-HD Zac-HD added the enhancement it's not broken, but we want it to be better label Aug 19, 2020
@Zac-HD Zac-HD self-assigned this Sep 9, 2020
@Zac-HD
Copy link
Member Author

Zac-HD commented Sep 10, 2020

master...Zac-HD:dtype-passthrough is working! I'm going to hold off on a PR until after #2615 and #2613 are merged though, because I don't need any more merge conflicts at the moment 🙂

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant