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

Customise nps.from_dtype() with keyword argument passthrough #2619

Merged
merged 2 commits into from
Sep 24, 2020

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Sep 13, 2020

When this can also be hooked up to arrays(..., elements=kwargs), it's almost surprising that it took us so long to think of - it's reasonably easy to implement, conceptually obvious, and very useful to generate arrays of all-finite or bounded elements without falling afoul of our precision-loss and overflow checks.

@rsokl: note that I moved our existing tests for from_dtype() to a new test module, as test_gen_data was getting way too large. You should therefore probably review the two commits separately.

The extension for complex_numbers() would be almost trivial, except that the strategy doesn't currently accept a width argument and I would prefer not to add that into the same change as everything else here. Happy to add that on request from someone who would actually use it though!

Closes #2552.

@Zac-HD Zac-HD added the enhancement it's not broken, but we want it to be better label Sep 13, 2020
@Zac-HD Zac-HD requested a review from rsokl September 13, 2020 07:13
@Zac-HD Zac-HD force-pushed the dtype-passthrough branch 3 times, most recently from b84a41e to 942b062 Compare September 16, 2020 03:39
Copy link
Contributor

@rsokl rsokl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good! As commented below, we should have tests for the arrays(dtype, elements=mapping) use case, since this will likely become one of the most popular patterns among the extra.numpy strategies.

Copy link
Contributor

@rsokl rsokl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Zac-HD Zac-HD merged commit bb02569 into HypothesisWorks:master Sep 24, 2020
@Zac-HD Zac-HD deleted the dtype-passthrough branch September 24, 2020 02:48
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 this pull request may close these issues.

Pass **kwargs through npst.from_dtype() to the inferred strategy
2 participants