Work on mapping variant cases #37

MonoidMusician · 2018-11-29T21:55:12Z

Fixes #36.

This is a work-in progress. In particular, I don't know of a way to come up with the correct map function for the VariantF that is generated from mapSome. Will we have to collect map dictionaries (corresponding to ro row), and then search for it in there?

Note: mapAll has a different implementation from mapSome only because the compiler doesn't solve Union r () r, and it is a constraint I don't want to propagate to the user. Otherwise it would be mapAll r = mapSome r case_.

Also need to write tests. All in good time 😉

MonoidMusician · 2018-11-30T17:01:19Z

Okay, I added an extensible version (mapSomeExpand r is like mapSome r expand but with more easily solved constraints), I finished up the corresponding functions for VariantF (using a new VariantFMaps), plus some tests, and I did some sanity checking around inference and type errors.

Just need better names and docs. 😁

natefaubion · 2018-12-02T17:59:22Z

Thank you for working on this. I haven't reviewed everything in detail, but I have a couple of API thoughts. The type of mapSome is suspiciously similar to on combinators. I think it would be nice to try and extend the existing vocabulary, and maybe have over as an alternative to on. I know you mentioned an issue with case_ which I haven't dived into, but my initial thoughts are that I'd like to be able to say:

case_
  # over _foo (\a -> show a)
  # over _bar (\a -> a + 1)

case_ # overMatch
  { foo: \a -> show a
  , bar: \a -> a + 1
  }

expand
  # over _foo ...
  # over _bar ...

expand # overMatch ...

natefaubion · 2018-12-02T18:47:38Z

I'm not super concerned about catering to the mapAll use-case. You very often want to exhaustively eliminate all the cases (so match vs case_ # onMatch makes sense to me), but I don't think it will be nearly as common to do the same for over to the extent that the extra chars are an inconvenience.

…to map-variant

MonoidMusician · 2021-06-29T21:24:06Z

I'm revisiting this PR since I'm looking at dhall-purescript again. I grepped my code and I only use expandOverMatch and expandTravMatch – I wonder if there might be a nicer API for that. I think type inference is important, which is why I did the tricks with Union. But I'm fairly happy with how it is now. What do you think?

Here's an example usage that I think is important to keep clean:
https://github.com/MonoidMusician/dhall-purescript/blob/0f331a5b1964f3a92113e964e985ce09811ef9ac/src/Dhall/Core/AST/Operations.purs#L21-L29

Also remove redundant variable `r4` and the corresponding constraint

thomashoneyman · 2021-12-17T17:01:50Z

src/Data/Variant.purs

+traverse
+  ∷ ∀ r rl ri ro r1 r2 r3 m
+  . RL.RowToList r rl
+  ⇒ VariantTravCases m rl ri ro


This is a small nit, but I think the library tends to use full names where possible, so I'd consider renaming this to VariantTraverseCases

thomashoneyman · 2021-12-17T17:02:04Z

src/Data/Functor/Variant.purs

+  coerceR = unsafeCoerce
+
+-- | Traverse over some labels (with access to the containers) and use
+-- | `travers f` for the rest (just changing the index type).


Suggested change

-- | `travers f` for the rest (just changing the index type).

-- | `traverse f` for the rest (just changing the index type).

thomashoneyman · 2021-12-17T17:03:25Z

src/Data/Variant.purs

@@ -28,8 +34,8 @@ import Data.Enum (class Enum, pred, succ, class BoundedEnum, Cardinality(..), fr
 import Data.List as L
 import Data.Maybe (Maybe)
 import Data.Symbol (class IsSymbol, reflectSymbol)
-import Data.Variant.Internal (class Contractable, class VariantMatchCases) as Exports
-import Data.Variant.Internal (class Contractable, class VariantMatchCases, class VariantTags, BoundedDict, BoundedEnumDict, VariantCase, VariantRep(..), contractWith, lookup, lookupCardinality, lookupEq, lookupFirst, lookupFromEnum, lookupLast, lookupOrd, lookupPred, lookupSucc, lookupToEnum, unsafeGet, unsafeHas, variantTags)
+import Data.Variant.Internal (class Contractable, class VariantMapCases, class VariantMatchCases) as Exports


Should this also re-export VariantTraverseCases?

thomashoneyman · 2021-12-17T17:04:58Z

I'm still looking through the constraints here, but I have reason to use over and traverse in an updated version of formless and they are working well there.

thomashoneyman · 2021-12-27T04:39:01Z

My usage of over and traverse can be seen in the (unreleased) Formless 3, where I'm using them to implement validation functions over variants (example: https://github.com/thomashoneyman/purescript-halogen-formless/blob/37e8a6adf63136fb9b10c5eff1f978e7599730f3/src/Formless.purs#L116). I think they're useful functions and deserve a place in variant!

natefaubion · 2021-12-31T22:08:18Z

I think if y'all are both satisfied with with the implementation/constraints and documentation, then I'm fine with y'all moving forward. I don't currently have any other constructive feedback.

thomashoneyman · 2022-02-28T22:53:35Z

Other than my naming and import comments I've mentioned above, this looks perfectly good to me.

thomashoneyman · 2022-03-03T17:22:03Z

@natefaubion I'm happy with the current state of things.

natefaubion · 2022-03-03T17:37:01Z

Release away!

MonoidMusician added 2 commits November 29, 2018 16:50

Work on mapping variant cases

df0bfac

Case mapping for VariantF, add extensible version, tests

c9f37b7

MonoidMusician added 9 commits December 29, 2018 22:43

Introduce over, rename map* to over*

9a2d6f9

Add traversal-type functions

9991a34

Exports

fd43160

Merge remote-tracking branch 'natefaubion/master' into map-variant

481fe28

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

06b060f

…to map-variant

Merge branch 'master' into map-variant

09a5e64

Reduce diff

db66919

Reduce diff again

4f77394

Improve documentation

b7df026

Rename functions ({over,traverse}{,One,Some}), better docs

4bafe7e

Also remove redundant variable `r4` and the corresponding constraint

thomashoneyman reviewed Dec 17, 2021

View reviewed changes

thomashoneyman mentioned this pull request Dec 27, 2021

Formless 3 thomashoneyman/purescript-halogen-formless#82

Merged

Address PR feedback

1aba110

thomashoneyman approved these changes Mar 3, 2022

View reviewed changes

thomashoneyman merged commit 131d7fb into natefaubion:master Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Work on mapping variant cases #37

Work on mapping variant cases #37

MonoidMusician commented Nov 29, 2018

MonoidMusician commented Nov 30, 2018

natefaubion commented Dec 2, 2018 •

edited

Loading

natefaubion commented Dec 2, 2018

MonoidMusician commented Jun 29, 2021

thomashoneyman Dec 17, 2021

thomashoneyman Dec 17, 2021

thomashoneyman Dec 17, 2021

thomashoneyman commented Dec 17, 2021

thomashoneyman commented Dec 27, 2021

natefaubion commented Dec 31, 2021

thomashoneyman commented Feb 28, 2022

thomashoneyman commented Mar 3, 2022

natefaubion commented Mar 3, 2022

	-- \| `travers f` for the rest (just changing the index type).
	-- \| `traverse f` for the rest (just changing the index type).

Work on mapping variant cases #37

Work on mapping variant cases #37

Conversation

MonoidMusician commented Nov 29, 2018

MonoidMusician commented Nov 30, 2018

natefaubion commented Dec 2, 2018 • edited Loading

natefaubion commented Dec 2, 2018

MonoidMusician commented Jun 29, 2021

thomashoneyman Dec 17, 2021

Choose a reason for hiding this comment

thomashoneyman Dec 17, 2021

Choose a reason for hiding this comment

thomashoneyman Dec 17, 2021

Choose a reason for hiding this comment

thomashoneyman commented Dec 17, 2021

thomashoneyman commented Dec 27, 2021

natefaubion commented Dec 31, 2021

thomashoneyman commented Feb 28, 2022

thomashoneyman commented Mar 3, 2022

natefaubion commented Mar 3, 2022

natefaubion commented Dec 2, 2018 •

edited

Loading