Continue `genericTraverse` work #366

tbagrel1 · 2022-02-04T14:51:14Z

The idea is to bring the open PR #349 on genericTraverse to a conclusion. I cannot edit it directly because it is a branch originating from a @treeowl's fork.

genericTraverse is an efficient implementation of traverse for
any Generic1 type.

treeowl · 2022-02-04T16:20:14Z

FYI, I'm emerging from the void, and hoping to get back into some of this stuff this week. But I don't oppose your assistance.

ConnorBaker · 2022-02-04T16:49:07Z

I mis-clicked and marked this as ready for review by accident -- trying to figure out how to undo that now.

treeowl · 2022-02-04T16:50:44Z

That's because GitHub's UI for that is awful. The "mark ready" is at the bottom, but the "convert to draft" is on the top of the right sidebar.

aspiwack · 2022-02-07T08:07:13Z

FYI, I'm emerging from the void, and hoping to get back into some of this stuff this week. But I don't oppose your assistance.

Hi David! Glad to hear from you again.

My suggestion is to use this rebased branch for you and Thomas to coordinate. Since this branch is hosted on the main repository and you both have commit rights, it seems like the easier solution.

tbagrel1 · 2022-02-07T08:19:19Z

At this point, on this branch, compared to @treeowl 's one,

I rebased to master and resolved merge conflicts
I changed D.xxx to Data.xxx as suggested by @aspiwack
I added the {-# OPTIONS_HADDOCK hide #-} pragmas

Here's the list of what might be required to merge this PR:

Public export of the modules ?
~~Write explicit export lists~~
Add comments and documentation, especially at the public module level (a docspec example would be nice)
See if we can use the latest release of linear-generics (0.2.0 if I'm not mistaken)

Feel free to amend this list, and select which items you want to tackle. @treeowl we can also set up a video call to sync up our work (email: firstname dot lastname at tweag dot io).

aspiwack · 2022-02-07T13:01:33Z

Write explicit export lists

You don't need explicit export lists on internal modules. But we ought to make sure that at least, all the classes which appear in the Haddock documentation is exported from somewhere non-internal. The gtraverse function ought to be exported from the same module which exports the Traversable type class.

tbagrel1 · 2022-02-08T10:07:35Z

I tried to address what I wrote in the TODO list yesterday.

The docspec that I added in Data.Functor.Linear is not working, because I don't know how to derive linear Generic1 on a custom data. I might need your help for this @treeowl.

treeowl · 2022-02-08T13:44:11Z

I expect to have time to look at it this evening.

tbagrel1 · 2022-02-09T14:52:48Z

Well, I still don't have a lot of luck with linear generics:

ghci> :{
ghci| data Pair a = MkPair a a
ghci| :}
ghci>  $(deriveGeneric1 ''Pair)

<interactive>:25:4: error:
    • Couldn't match type ‘[template-haskell-2.17.0.0:Language.Haskell.TH.Syntax.Dec]’
                     with ‘template-haskell-2.17.0.0:Language.Haskell.TH.Syntax.Exp’
      Expected: template-haskell-2.17.0.0:Language.Haskell.TH.Lib.Internal.ExpQ
        Actual: template-haskell-2.17.0.0:Language.Haskell.TH.Syntax.Q
                  [template-haskell-2.17.0.0:Language.Haskell.TH.Syntax.Dec]
    • In the expression: deriveGeneric1 ''Pair
      In the untyped splice: $(deriveGeneric1 ''Pair)

Did I do anything wrong? (using linear-generics-0.2)

treeowl · 2022-02-09T15:02:35Z

I think GHCi may not support declaration splices at all. So you can do that in a module, but I think not in the interpreter. The ViaGHCGenerics stuff should work anywhere, but use the latest (major) version of linear-generics because it's different (much better) than what came before. Don't expect that deriving method to be too efficient, but it'll work for playing around just fine.

aspiwack

I think that all we need is an example use in the examples/ directory. Let's call it Simple/Deriving.hs.

Then we can merge.

src/Data/Functor/Linear.hs

`genericTraverse` is an efficient implementation of `traverse` for any `Generic1` type.

+ Public export of (linear) generic traverse through `Data.Functor.Linear` + Add comments and documentation (including a docspec) + Move from `linear-generics-0.1.0.1` to `linear-generics-0.2` + Format the codebase

examples/Generic/Traverse.hs

aspiwack · 2022-02-10T17:04:31Z

examples/Generic/Traverse.hs

+  deriving (Show, Prelude.Eq)
+
+instance Control.Functor WithLog where
+  fmap f (WithLog lx x) = WithLog (logApp Nothing lx) (f x)


For what it's worth, this doesn't verify the Functor laws, because, for instance, fmap id is not the identity (as it get something logged). You can get away with it by asserting that the log string is not actually meaningful.

Honestly, though, I'd just use an applicative from linear-base.

But we don't have many Control.Applicative instances in linear-base. I only see Identity, IO, ReaderT, StateT, StateR, and (,) a. Which one would make a good example in your opinion?

(,) a is a pretty obvious candidate!

Or StateT Id. But (,) a is likely to be the better choice.

Sorry, I got confused for a moment. I thought you were looking for a Traversable instance to derive. For an appropriate Applicative for testing, is there such a thing as a free control applicative? Free structures tend to make good test cases.

We don't have one defined. We probably could, but on the other hand, it could not be printed or compared for equality. I don't think it would actually make for a good test case.

The goal of this file is primarily to serve as a code example for documentation purposes. I think that using writer is fine. It's a perfectly serviceable applicative structure.

treeowl · 2022-02-11T07:46:38Z

BTW, the reason I haven't hopped back into this as I promised is that I woke up good and sick Monday. Ugh.

aspiwack · 2022-02-11T07:49:27Z

BTW, the reason I haven't hopped back into this as I promised is that I woke up good and sick Monday. Ugh.

Oh 🙁 . I hope you are feeling better.

ConnorBaker marked this pull request as ready for review February 4, 2022 16:47

ConnorBaker marked this pull request as draft February 4, 2022 17:05

tbagrel1 force-pushed the tbagrel1/generic-traverse branch from c2ca9ad to 8953828 Compare February 8, 2022 10:11

tbagrel1 marked this pull request as ready for review February 8, 2022 10:14

aspiwack reviewed Feb 10, 2022

View reviewed changes

src/Data/Functor/Linear.hs Outdated Show resolved Hide resolved

src/Data/Functor/Linear.hs Outdated Show resolved Hide resolved

src/Data/Functor/Linear.hs Outdated Show resolved Hide resolved

treeowl and others added 2 commits February 10, 2022 16:16

Add genericTraverse

f397370

`genericTraverse` is an efficient implementation of `traverse` for any `Generic1` type.

Public export, documentation, linear-generics-0.2

233c9ea

+ Public export of (linear) generic traverse through `Data.Functor.Linear` + Add comments and documentation (including a docspec) + Move from `linear-generics-0.1.0.1` to `linear-generics-0.2` + Format the codebase

tbagrel1 force-pushed the tbagrel1/generic-traverse branch from 8953828 to 2e072b4 Compare February 10, 2022 16:34

aspiwack reviewed Feb 10, 2022

View reviewed changes

tbagrel1 force-pushed the tbagrel1/generic-traverse branch from 2e072b4 to 682d85f Compare February 11, 2022 08:26

Add example for genericTraverse

e096e60

tbagrel1 force-pushed the tbagrel1/generic-traverse branch from 682d85f to e096e60 Compare February 11, 2022 13:55

tbagrel1 mentioned this pull request Feb 11, 2022

Add generic implementation and generic deriving for linear Semigroup and Monoid #371

Open

aspiwack added this to the v0.2.0 milestone Feb 11, 2022

aspiwack approved these changes Feb 14, 2022

View reviewed changes

tbagrel1 merged commit 6d973b2 into master Feb 14, 2022

tbagrel1 deleted the tbagrel1/generic-traverse branch February 14, 2022 14:38

tbagrel1 mentioned this pull request Feb 15, 2022

Add genericTraverse #349

Closed

Continue genericTraverse work #366

Continue genericTraverse work #366

Uh oh!

Conversation

tbagrel1 commented Feb 4, 2022

Uh oh!

treeowl commented Feb 4, 2022

Uh oh!

ConnorBaker commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

treeowl commented Feb 4, 2022

Uh oh!

aspiwack commented Feb 7, 2022

Uh oh!

tbagrel1 commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aspiwack commented Feb 7, 2022

Uh oh!

tbagrel1 commented Feb 8, 2022

Uh oh!

treeowl commented Feb 8, 2022

Uh oh!

tbagrel1 commented Feb 9, 2022

Uh oh!

treeowl commented Feb 9, 2022

Uh oh!

aspiwack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aspiwack Feb 10, 2022

Choose a reason for hiding this comment

Uh oh!

tbagrel1 Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

treeowl Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

aspiwack Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

treeowl Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

aspiwack Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

treeowl commented Feb 11, 2022

Uh oh!

aspiwack commented Feb 11, 2022

Uh oh!

Uh oh!

Continue `genericTraverse` work #366

Continue `genericTraverse` work #366

ConnorBaker commented Feb 4, 2022 •

edited

Loading

tbagrel1 commented Feb 7, 2022 •

edited

Loading