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

Remove Arbitrary? #65

Open
paf31 opened this issue Jan 7, 2017 · 17 comments
Open

Remove Arbitrary? #65

paf31 opened this issue Jan 7, 2017 · 17 comments
Labels
purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 type: breaking change A change that requires a major version bump.

Comments

@paf31
Copy link
Contributor

paf31 commented Jan 7, 2017

I'm adding this to track the discussion going on in #63.

The argument for removing Arbitrary is that instances are almost never canonical, so we almost always end up needing newtypes.

A possible downside is that we'd still need a class like Arbitrary if we want to derive generators using generic-rep. That means that we'd need some way to reflect a Gen as an instance, or we're back to Arbitrary instances anyway. CoArbitrary is also much nicer with type classes.

@matthewleon
Copy link
Contributor

I've created a branch for quickcheck without typeclasses here: https://github.com/matthewleon/purescript-quickcheck/tree/no-arbitrary

It works, but needs to be made a bit friendlier.

I'm going to try addressing the challenges you bring up in this post in it and see what I come up with. I'll also make some branches of libs that use quickcheck that use the aribtrary-less version to see what problems are caused and how they can be solved.

@hdgarrood
Copy link
Contributor

There's also https://github.com/jystic/purescript-jack, which I think is already a fairly advanced quickcheck-style library with no Arbitrary type class (so I'm not actually sure if it's worth us going to a lot of effort to move this library in that direction since it seems jack has already done it really nicely). I think the automatic shrinking is a huuuge plus. I should add, though, that I haven't gotten around to trying it out properly yet.

@matthewleon
Copy link
Contributor

Thanks for bringing that up. Very good points. I'll give my branch a rest then :)

@JordanMartinez JordanMartinez added purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 type: breaking change A change that requires a major version bump. labels Dec 4, 2021
@JordanMartinez
Copy link
Contributor

Jack is no longer maintained (AFAIK). Moreover, we added versions of quickcheck-laws that just use a generator rather than Arbitrary. I'd say removing Arbitrary altogether would be good.

@paf31
Copy link
Contributor Author

paf31 commented Dec 4, 2021

How would you implement generic arbitrary?

@JordanMartinez
Copy link
Contributor

Meaning, an arbitrary for the generic representations of a type via the Generic type class? Hm... Good question.

@paf31
Copy link
Contributor Author

paf31 commented Dec 4, 2021

@JordanMartinez
Copy link
Contributor

Ah...

@garyb
Copy link
Member

garyb commented Dec 4, 2021

You could have a MonadGen m => Generic a => m a I think.

@paf31
Copy link
Contributor Author

paf31 commented Dec 4, 2021 via email

@hdgarrood
Copy link
Contributor

I think removing the Arbitrary class only makes sense if we are prepared to lose genericArbitrary along with it. If Arbitrary instances are almost never canonical, then surely it's not a good candidate for automatic derivation either?

@paf31
Copy link
Contributor Author

paf31 commented Dec 6, 2021

I don't think the need for newtypes makes Arbitrary a bad type class. Yes, the instances are rarely canonical, but that's just what newtypes solve - they make non-canonical things canonical by naming them for that purpose. Arbitrary needs a lot of newtypes in order to be used in a principled way, but that's the trade off you have to make for its convenience. If you're already naming everything with newtypes in a principled way, then you get the ability to use things like genericArbitrary, which can be very convenient. And with coerce, the cost of this kind of approach should go down.

@hdgarrood
Copy link
Contributor

Right - I guess what I'm saying is, if we think genericArbitrary is useful, then to me, that's sufficient to say we'll keep Arbitrary. I do think there's a coherent argument along the same lines as the one against Arbitrary which says that we should get rid of genericArbitrary as well, and I can't think of a time I've used genericArbitrary, but I am also sympathetic to the argument that it is very convenient and I would be fine with leaving Arbitrary in for that reason.

@JordanMartinez
Copy link
Contributor

I haven't used genericArbitrary, but that's likely because I didn't realize it existed. Considering these two examples...

data SumType = A | B | C | D | E | ...
derive instance Generic SumType _
instance Semigroup SumType ...
instance Monoid SumType ...

quickCheckGen do
   (a :: SumType) <- genericArbitrary
   pure $ a <> mempty == a

rather than

data SumType = A | B | C | D | E | ...
instance Semigroup SumType ...
instance Monoid SumType ...

quickCheckGen do
  a <- oneOf [ A, B, C, D, E, ... ]
  pure $ a <> mempty == a

The first is easier to write because I don't need to provide each constructor to oneOf. However, as soon as I want to tweak how frequently an A appears, now the second version is more flexible.

If I wanted the same flexibility via newtypes and genericArbitrary, how would that work? And would it ultimately be less work than just writing it manually?

On another hand, how is genericArbitrary intended to be used? Is it for writing another type's Arbitrary instance?

@paf31
Copy link
Contributor Author

paf31 commented Dec 6, 2021

You wouldn't use genericArbitrary in the second case. It's not meant to replace every possible Arbitrary instance you'd want to write. You'd use it as a convenient shortcut to begin, and stop using it once it no longer matches your use case.

@JordanMartinez
Copy link
Contributor

Gotcha. Then what other newtypes would need to be added to make it possible to use Arbitrary in a principled way?

@JordanMartinez
Copy link
Contributor

Given the points made by @paf31 above, I think this issue should be closed and Arbitrary kept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 type: breaking change A change that requires a major version bump.
Projects
None yet
Development

No branches or pull requests

5 participants