purescript-validation icon indicating copy to clipboard operation
purescript-validation copied to clipboard

Suggestion: remove Semiring Validation from core

Open hdgarrood opened this issue 5 years ago • 7 comments

For some code to be part of the core libraries, I think there should be quite a high bar to meet. The core team should be able to understand and maintain it, it should be robust and correct, and it should be something we expect people to want to use regularly in their programs. I'm not sure Data.Validation.Semiring meets that bar. I certainly don't understand it - I don't understand why you would want to be able to combine errors in two different ways like that. It has also proven resistant to efforts to document it. Additionally, the type which you're meant to use for accumulating errors, Data.Semiring.Free, isn't even a real Semiring: https://github.com/purescript/purescript-semirings/issues/11.

Given all of these things, I think Data.Validation.Semiring (and probably Data.Semiring.Free too) would be best moved to a separate non-core library.

hdgarrood avatar Oct 01 '20 12:10 hdgarrood

Makes sense to me, but where would it go? purescript-contrib?

Also, do any other purescript libs depend on this repo?

JordanMartinez avatar Oct 02 '20 00:10 JordanMartinez

I'm sure other libraries depend on this library, but I imagine that the vast majority of those only depend on Data.Validation.Semigroup, not the Semiring part. I'd suggest not putting it in contrib unless we can identify a maintainer who does feel able to maintain it; I feel like moving it to someone's personal account might be best. @paf31 @cryogenian do either of you have thoughts on this?

hdgarrood avatar Oct 02 '20 19:10 hdgarrood

The semiring structure on errors is required to satisfy the annihilation law (empty <*> rhs = empty) of Alternative:

Left zero <*> Left failure = Left (zero * failure) = Left zero

A semigroup is insufficient:

Left zero <*> Left failure = Left (zero <> failure) = Left failure

Data.Validation.Semigroup.V has no Alt nor Plus instances, but they could be lawful. Neither type can implement Alternative though because they don’t satisfy the distributivity law ((f <*> a) <|> (g <*> a) = (f <|> g) <*> a), but Data.Validation.Semiring.V could if we dropped the law (which seems to be the direction we’re taking in https://github.com/purescript/purescript-control/issues/63).

So I think that whether to keep Data.Validation.Semiring.V or not might depends on what we do about Alternative and its laws?

Also, is there any reason for not having Alt and Plus (unless we merge Plus into Alternative) instances for Data.Validation.Semigroup.V?

kl0tl avatar Nov 26 '20 17:11 kl0tl

I don't really understand what you're getting at, is it that you think we should consider keeping the Semiring validation because it might be able to support an Alternative instance (whereas the Semigroup version can't)? For me that's very much a secondary concern after the concerns I raised in my initial comment.

hdgarrood avatar Dec 14 '20 02:12 hdgarrood

Ps-routing from contrib uses semiring. Maybe move it to contrib or even make it part of routing lib, if noone needs it?

cryogenian avatar Dec 14 '20 09:12 cryogenian

I don't really understand what you're getting at, is it that you think we should consider keeping the Semiring validation because it might be able to support an Alternative instance (whereas the Semigroup version can't)?

I don’t have much of an opinion about this, I just wanted to document a subtle difference between the two Validation types ^^

kl0tl avatar Dec 14 '20 23:12 kl0tl

I wasn't aware that purescript-routing uses semiring validation, thanks for making me aware of that. At least we have a motivating example now. I'd like to investigate whether using semiring validation helps produce better errors and if the fact that Data.Semiring.Free isn't a real semiring causes any issues for the routing library; I feel like we ought to have an answer for what will happen to the routing library before going ahead with this. The easiest thing would of course be to put semiring validation and Data.Semiring.Free back inside the routing library (if you go far back enough in the git history of the routing library it appears that it originated inside there), but perhaps investigating these things will help us better understand/motivate semiring validation.

hdgarrood avatar Dec 15 '20 06:12 hdgarrood