Add record elimination, tests and docs #6

MonoidMusician · 2017-07-11T18:27:59Z

Okay here's my stab at it.

The compiler insists that class VariantRecordElim, class VariantMatch must be exported – is this true? it seems a bit ugly ...
I just used StrMap.lookup, would you prefer FFI?
Let me know if you want anything renamed, or if you want the helper classes merged, etc.

And of course typeclass polymorphism doesn't work from inside records, but I can't even begin to think of how to accomplish that. Otherwise it seems to work well!

natefaubion · 2017-07-12T13:19:17Z

src/Data/Variant.purs

+    , RowToList record rlist
+    , VariantMatch vlist rlist result
+    , ListToRow vlist variant
+    , ListToRow rlist record )


I don't think these ListToRows are necessary.

@LiamGoodacre was suggesting it helps with the bidirectional fundeps. I can play around with it, see what makes the compiler happy.

I think they are: in different situations either variant or record are being computed - you need something to produce what form the other should take (neither RowToList nor VariantMatch do this). If you only consider them both together as "input", then you won't need the ListToRow. But then your type inference will suck.

natefaubion · 2017-07-12T13:23:36Z

src/Data/Variant.purs

+-- | that even `id` must be given a type annotation to be used in this way,
+-- | since it normally depends on `Category`: use `id :: forall a. a -> a` not
+-- | `id :: forall a c. Category c => c a a`.
+elim


Thank you for documenting the limitation well. I don't think there's anything to be done about it right now. Maybe when we get instance chains and constraint kinds, there's something we could do about it, but I'm not sure.

Maybe fold or foldVariant would be a better name? We don't really use elim in the ecosystem, but folds are quite common.

Can we also write this for Data.Functor.Variant?

Also, for a canonical fold, the eliminator should come first. This works much better with currying. I don't mind having a flipped variant, but just like case_ takes the variant last, fold should take the variant last.

I think my comments are actually incorrect, and the limitation mostly applies to let bindings and records written inline, which seem to follow different inference rules. Maybe I should reduce the note to, if you're getting an ambiguous type error, move the declaration up a level or two or give it an explicit type ...

I don't like fold as it's just one value being applied to one function – got any other ideas? handleMany[Cases]? match?

That's like saying an uncurried form of foldr isn't a list fold :) A record to handle all the possible cases is precisely the fold for a given closed variant.

I'm crossing wires a bit :) The signature for foldList I gave is the fold for lists, whether or not it's represented with Variant or data constructors or Church-encoding. It's not possible to define a definition of List with Variant without a newtype or VariantF + Mu. If I were to use VariantF, it might be:

type ListF a f = VariantF (nil :: FProxy (Const Unit), cons :: FProxy (Tuple a))) f type List a = Mu (ListF a)

In which case, the proposed record eliminator would be the actual fold when run with cata (a generalized folding operation).

I get that Variant encodes sum types, but I think that elim is more of a matching operation, and in order to get a folding operation you have to add something extra – aka cata (or explicit recursion) – so I wouldn't say that elim itself is a fold. (Again, it only applies one function to one value, there's no default value equivalent to like fold Nothing == mempty, and so on.)

So I guess I'm leaning towards match or cases as a name.

case _ of Nil -> Nothing Cons h t -> Just h match { nil: \_ -> Nothing -- const Nothing , cons: \(Tuple h t) -> Just h -- Just <<< fst }

A fold is just case analysis (which is why I suggested it as an alternative to elim). foldr :: (a -> r -> r) -> r -> List a -> r is merely case analysis for lists (an argument to handle Cons and an argument to handle Nil, its not about defaults). For real world examples of this, look at purescript-argonaut-core. That said, I'm fine with match 😛. It's as good a name as any, and probably more what people would expect :)

Anecdata: I'd call this fold. I guess match works too 😉.

Anecdata.

Ooh, nice dime word. Anyways, let's call it match (thanks @cryogenian) and I'll stop arguing 😂 .

natefaubion · 2017-07-12T13:24:33Z

src/Data/Variant.purs

+  , rlist → vlist result
+instance variantMatchNil
+  ∷ VariantMatch R.Nil R.Nil r
+instance variantMatchCons


Anything that shows up in a public signature must be exported. I think we should move these to a Data.Variant.Internal module, probably.

natefaubion · 2017-07-12T13:26:33Z

src/Data/Variant.purs

+elim v r =
+  case coerceV v of
+    Tuple tag a →
+      a # unsafePartial fromJust


I would like to see what happens with doolse/purescript-records#1. I don't mind StrMap for a proof of concept, but I don't like to rely on it.

okay, I'll keep on top of releases of purescript-record now, looking forward to it!

MonoidMusician · 2017-07-12T21:36:49Z

I was playing around with RowList and I think I found a nice abstraction: https://gist.github.com/MonoidMusician/fe7d35c437c23833a5ca5b67a5ca3744

We could maybe put the classes in typelevel and just use them in variant, what do you think about that?

The next layer of abstraction would be a ZipWith kind of thing, for applyRecord and such, since we really are just matching on corresponding rows and checking a relation.

Grant and pick

I decided to change the name of elim to grant, since the Variant is giving it's value to a Record, and now there's a dual(?) called pick where the Variant decides which row in the Record it gets applied to. Does that make sense? Basically the difference is where the function is – Variant or Record.

LiamGoodacre · 2017-07-12T23:24:44Z

src/Data/Variant.purs

+    (record ∷ # Type)
+    result
+  | variant result → record
+  , record → variant result


I think these fundeps should be | variant -> record result, record -> variant result as the instances (delegated to rowlists) only match on the variant and record - and only one of these is sufficient to infer the structure for the other. We never match on result, so it can always be determined.

Actually, this type class could have | -> variant record result, but the other one should have | vlist -> rlist result, rlist -> vlist result.

The reason why | -> variant record result makes sense here is that there is only ever one instance that doesn't even pattern match on it's parameters.

I played around with this a bit, and I couldn't find any difference between the fundep signatures – the same examples seem to compile with either signature, so I've taken your suggestions, I just don't understand why it works (yet!).

…to elim

MonoidMusician · 2017-07-24T19:10:07Z

Okay, I think this is ready to be merged – thanks for the unsafeGet PR!

natefaubion · 2017-07-26T02:33:39Z

src/Data/Variant/Internal.purs

+-- | Checks that a `RowList` matches the argument to be given to the function
+-- | in the other `RowList` with the same label, such that it will produce the
+-- | result type.
+class RLMatch


These typeclass names surface when there's a type error with one of the branches. Do you think we could change it to something more descriptive than RLMatch?

Could not match type Int with type String while solving type class constraint Data.Variant.Internal.RLMatch t3 t4 String

Something like VariantMatchBranch, maybe?

Okay, good point! It's a pity that this is overlapping though:

instance variantMatchAFail ∷ Fail (TypeConcat "Unhandled variant case " (TypeConcat sym (TypeConcat " of type " (TypeString a)))) ⇒ VariantMatchEachCase (R.Cons sym a v) R.Nil res

[1/1 OverlappingInstances] Overlapping type class instances found for Data.Variant.Internal.VariantMatchEachCase (Cons "bar" String (Cons "baz" Boolean (Cons "foo" Int Nil))) Nil String The following instances were found: Data.Variant.Internal.variantMatchAFail (chosen) Data.Variant.Internal.variantMatchCons Data.Variant.Internal.variantMatchNil [1/1 NoInstanceFound] test/Variant.purs:62:14 v 62 match' = match 63 { } ^ A custom type error occurred while solving type class constraints: Unhandled variant case bar of type String

Looks nice, but it only seemed to work when the record is empty ... Hopefully the errors with Cons are sufficiently obvious.

natefaubion · 2017-07-28T15:51:01Z

Thanks for the PR!

MonoidMusician · 2017-07-28T15:52:59Z

Thank you!

Add record elimination, tests and docs

8f63e78

cryogenian mentioned this pull request Jul 12, 2017

Case struct #7

Closed

natefaubion requested changes Jul 12, 2017

View reviewed changes

LiamGoodacre reviewed Jul 12, 2017

View reviewed changes

MonoidMusician force-pushed the elim branch from 8f63e78 to 5af9666 Compare July 16, 2017 03:02

Merge remote-tracking branch 'origin/master' into elim

74727e4

MonoidMusician force-pushed the elim branch from 5af9666 to 74727e4 Compare July 18, 2017 17:52

MonoidMusician and others added 9 commits July 18, 2017 12:54

Merge branch 'master' of github.com:natefaubion/purescript-variant in…

3edabc2

…to elim

Update names (elim -> match), documentation

14fc983

Reorder match after on, case_, default

09bd62b

VariantF match

ca715ac

(Un)flip match

7c58c7c

Fundeps

a5d04f3

Use purescript-record

26c3746

Use Data.Record.unsafeGet

a129ca3

Use RProxy, RLProxy from Type.Row

0548d9e

natefaubion reviewed Jul 26, 2017

View reviewed changes

Rename classes

40be31e

natefaubion approved these changes Jul 28, 2017

View reviewed changes

natefaubion merged commit 2e3fd81 into natefaubion:master Jul 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add record elimination, tests and docs #6

Add record elimination, tests and docs #6

MonoidMusician commented Jul 11, 2017

natefaubion Jul 12, 2017

MonoidMusician Jul 12, 2017

LiamGoodacre Jul 12, 2017 •

edited

Loading

natefaubion Jul 12, 2017

natefaubion Jul 12, 2017

natefaubion Jul 12, 2017

MonoidMusician Jul 16, 2017

natefaubion Jul 17, 2017

natefaubion Jul 17, 2017 •

edited

Loading

MonoidMusician Jul 17, 2017

natefaubion Jul 18, 2017

garyb Jul 18, 2017

MonoidMusician Jul 18, 2017

natefaubion Jul 12, 2017

natefaubion Jul 12, 2017

MonoidMusician Jul 16, 2017

MonoidMusician commented Jul 12, 2017

LiamGoodacre Jul 12, 2017 •

edited

Loading

LiamGoodacre Jul 12, 2017

LiamGoodacre Jul 12, 2017

MonoidMusician Jul 16, 2017

MonoidMusician commented Jul 24, 2017

natefaubion Jul 26, 2017

natefaubion Jul 26, 2017

MonoidMusician Jul 26, 2017

natefaubion commented Jul 28, 2017

MonoidMusician commented Jul 28, 2017

Add record elimination, tests and docs #6

Add record elimination, tests and docs #6

Conversation

MonoidMusician commented Jul 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiamGoodacre Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natefaubion Jul 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MonoidMusician commented Jul 12, 2017

Grant and pick

LiamGoodacre Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MonoidMusician commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natefaubion commented Jul 28, 2017

MonoidMusician commented Jul 28, 2017

LiamGoodacre Jul 12, 2017 •

edited

Loading

natefaubion Jul 17, 2017 •

edited

Loading

LiamGoodacre Jul 12, 2017 •

edited

Loading