search

BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.32k stars 144 forks source link

Add Gen::gen_uniform #283

Open audunska opened 3 years ago

audunska commented 3 years ago

Arbitrary impls for nested data often needs access to some kind of random numbers. To avoid exposing rand types, a simple uniformly distributed float on the unit interval is often sufficient, as many distributions over floats and integers can be generated from this.

See #279.

kstrafe commented 3 years ago

Why can't we have gen_range instead to generate a uniform range? 0f32..=3f32 does not expose a rand type for instance.

kstrafe commented 3 years ago

Another note: Scaling up a uniform f32 [0, 1) range to [1, 10) via x*9 + 1 leaves holes. There exists no float in [0, 1) that maps to 7.256350994110107421875

akesling commented 3 years ago

Why can't we have gen_range instead to generate a uniform range? 0f32..=3f32 does not expose a rand type for instance.

Why not just make the existing Gen::gen_range public? It's already used extensively in the provided Arbitrary implementations to good effect.

EDIT: I should have read up the comment chain first. https://github.com/BurntSushi/quickcheck/issues/267#issuecomment-757022188 already discusses gen_range being made public. The conclusion (as implied by the OP) is that it is apparently undesirable to make rand a public dependency.

BurntSushi commented 3 years ago

Because it introduces a public dependency on rand.

akesling commented 3 years ago

Because it introduces a public dependency on rand.

@BurntSushi Have you considered updating included implementations of Arbitrary to only use the public interface?

Doing so should help clarify by example how people should go about creating their own impls. Having "internal" implementations use gen_range and then not providing it publicly was... disappointing :(.

BurntSushi commented 3 years ago

I've already acknowledged the deficiency elsewhere. I'm not ignorant of the problem. Ignorance isn't what is holding back improvement here. Time to dedicate toward maintenance is.

akesling commented 3 years ago

I've already acknowledged the deficiency elsewhere. I'm not ignorant of the problem. Ignorance isn't what is holding back improvement here. Time to dedicate toward maintenance is.

@BurntSushi apologies if I've offended. I'm the ignorant participant in this thread just trying to understand the state of the world to use the library (and maybe help others do the same). Do you have a link to discussion of changing the included implementations of Arbitrary? The more cross-linked these discussions become, the harder it is for people like me to ask stupid questions.

Also, would you be open to a PR that migrates some Arbitrary impls to only the public interface?

BurntSushi commented 3 years ago

Again, the problem here is my available time to do maintenance. Organizing discussions is maintenance. And changing the Arbitrary impls to use only public APIs is not only impossible but a waste of my time (and also requires the time to do maintenance). That is, I don't know what benefit it would bring. Unless you're proposing adding new APIs to Gen, in which case, that could be worthwhile, yes. But you'll want to see my comments on this topic in recent PRs and issues first.

I'm sorry but you'll have to look at recently opened issues and PRs to understand the state of the world here. There aren't that many.

BurntSushi commented 3 years ago

I made a couple of edits to my last comment in case you're reading via email, FYI.

akesling commented 3 years ago

For anyone who intersects with this discussion and needs the context I was lacking, start by reading https://github.com/BurntSushi/quickcheck/issues/241 and https://github.com/BurntSushi/quickcheck/pull/265 .

As it seems I've overstayed my welcome in this thread, I'll do my best to read a proper history of all of this (which I tried but apparently failed to do before posting initially). Once there, I'll comment on whatever open PR/Issue is more pertinent, or, as per the request in https://github.com/BurntSushi/quickcheck/pull/265, open a more relevant issue.

@BurntSushi thank you for taking the time to help contextualize all of this. I'm a fan of your work and can appreciate the burden of so much public code to manage.

BurntSushi commented 3 years ago

OK, I'm finally at a workstation.

I personally think this is the most promising direction: https://github.com/BurntSushi/quickcheck/issues/277

There is even a PR for it: https://github.com/BurntSushi/quickcheck/pull/278

I just haven't had time to sit down, digest and commit to it. That's what I meant earlier by the problem being bottlenecked on me and my time.