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

Add Haddocks & doctests (WIP) #73

Closed
wants to merge 3 commits into from

Conversation

ocharles
Copy link

@ocharles ocharles commented Jan 30, 2019

This pull request aims to add Haddocks to all exported symbols. I've made a good start, but would like feedback before continuing.

Personal To Do List

  • Drop build-type: Custom
  • Move cata, ana, hylo documentation/examples to fold, unfold, refold.
  • Drop prepro example, link to a blog post.
  • Replace references to "pattern functor" with "base functor".

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

I read the title, and I'm super excited, let's see the contents...

@@ -849,3 +1114,12 @@ instance (GCoerce f g, GCoerce f' g') => GCoerce (f :*: f') (g :*: g') where
instance (GCoerce f g, GCoerce f' g') => GCoerce (f :+: f') (g :+: g') where
gcoerce (L1 x) = L1 (gcoerce x)
gcoerce (R1 x) = R1 (gcoerce x)

-- $intro
-- TODO
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I want to provide a little intro to what this library is about, and the fundamental concepts users should know about. I'll also provide references to good literature for further reading.

-- TODO

-- $patternFunctors
-- TODO
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer yaya's term of "pattern functor" rather than "base" functor. How do the authors feel about this? Either way, this section should explain what base/pattern functors are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code has Base, and I'd avoid using many names for same concept.

Also pattern is so overloaded: Meijer 1991 calls recursion-schemes recursion patterns, we have pattern matching, ..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, calling them pattern functors and then having the connection made with Base is even more confusing. I don't like the name Base but it's probably not worth the breaking API change now. Will change this to "base functors".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code has Base, and I'd avoid using many names for same concept

When we make the big breaking gather-scatter change, maybe we can include a few other breaking name changes?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do, here are the names I would like to suggest: "recursive type" for [a], "non-recursive type" or "single step type" for ListF a r (and I'd like to use r for recursive positions instead of whatever's the next available letter, in this case b), "recursive function" for cata f, and "non-recursive function" for f itself. Having a consistent name for f across recursion schemes would be especially useful for writing the part of the documentation which talks about multiple recursion schemes, as the current documentation uses a slightly different name in each one: f-algebra, f-coalgebra, f-w-algebra...

Copy link
Contributor

@phadej phadej Jan 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need explicit examples of

which require both Recursive and Corecursive instances for a type (which is generally unsafe to have)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m aware of the licensing issues involved, no worries.

The general unsafety of folding lazy (corecursive) structures and unfolding to strict structures is pretty well established. E.g.,

let nats = 0 : fmap (+ 1) nats
in foldr (+) 0 nats

won’t terminate. In general, only Base t ~ Const t has safe instances of both. The separation of safe and unsafe instances is probably the primary motivating force behind yaya.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But let me phrase that frustration a different way – I don’t think there are any laws that relate project to cata, but there are laws for cata on its own, as well as laws that relate project to embed and laws that relate embed to cata. So the project in Recursive is lawless, whereas embed in Recursive wouldn’t be.

Copy link
Contributor

@phadej phadej Jan 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sellout so you argue that having Projectable and Steppable (Why not Embeddable?) for data Stream a = a :> Stream a is ok, but we shouldn't have Recursive for it?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so as I mentioned, yaya is still experimenting with a bunch of stuff, and the type class structure is part of that. The law-introducing classes are

  • cata
  • ana
  • embed + project
  • embed + cata
  • project + ana

Ideally, I would like to avoid having any lawless type classes, so I had provided

  • cata (Recursive)
  • ana (Corecursive)
  • embed + project (Steppable)

as three separate classes. But, I’ve run into cases where embed is impossible to implement, so I ended up pulling out Projectable so I could still implement project in that case. However, Projectable is lawless. You need to either implement Steppable or Corecursive to introduce laws on it. So I’d rather not have it exist. So it might be better to move to the

  • embed + cata
  • project + ana

classes. Still waffling. I’d kind of like to split each operation into its own class, if I could somehow require that the lawless classes had one of their “companion” classes implemented.

-- TODO

-- $fixedPoints
-- TODO
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I will briefly explain the concept of fixed points wrt types.

@@ -181,29 +301,77 @@ class Functor (Base t) => Recursive t where
-> a
gprepro k e f = extract . c where c = fmap f . k . fmap (duplicate . c . hoist e) . project

-- | TODO
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what all the dist stuff is about. Could someone enlighten me?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README provides a quick intro:

One way to implement such a custom recursion scheme is to combine the features of existing recursion schemes. For example, a "paramorphism" gives the non-recursive function access to the original sub-trees, a "zygomorphism" gives that function access to auxiliary results computed from those sub-trees, and so the combined "zygomorphic paramorphism" gives that function access to both the original sub-trees and the auxiliary results. In order to construct such combinations, most of our recursion schemes come in a generalized variant, e.g. gzygo, and in a "distributive law transformer" variant, e.g. distZygoT. Just like monad transformers, distributive law transformers can be combined into stacks, and like monad transformers, the order in which you combine the layers determines how the layers interact with each other. Apply a generalized recursion scheme to a stack of distributive laws in order to obtain a recursion scheme which has both the features of the generalized recursion scheme and those of the distributive laws.

It's based on a paper called "recursion schemes from comonads".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that a comonad is almost enough to define a recursion scheme, and although they don't say this in the paper, that's appealing because we have comonad transformers, so maybe we can combine comonads in order to combine recursion schemes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what's the relationship between comonads and recursion schemes? Well, consider zygo: compared to cata, we're carrying around an extra (b,). That's a comonad. And consider histo: compared to cata, we're carrying around an extra Cofree (Base t); that's another comonad.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea behind gcata is that it generalizes cata, zygo, histo, and so on, and we can choose which one by instantiating its w comonad to Identity, (b,), or Cofree (Base t). We can also instantiate it to other comonads, or to stacks of comonad transformers, in order to obtain novel recursion schemes.

Except it turns out a comonad is not enough! We need a distributive comonad. That's where the distributive laws come in.

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

First, let's not make this package build-type: Custom. We (me and Samuel) discussed this, and we have Makefile to run the test. You need cabal new-build and doctest compiled with GHC you use to new-build. It's a little trouble to developers, but the win is one less build-type: Custom.

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

I value the effort but I think we need to think of a better structure. Interspersing "API reference" with "Tutorial" makes both crappier, then when they are separate.

Main pain point: you cannot see an overview of what is in Recursive class.

@ocharles
Copy link
Author

Main pain point: you cannot see an overview of what is in Recursive class.

Well the biggest pain point here is that Haddock duplicates the documentation because of the generic definition. I'm open to suggestions here, but I do think all schemes at least need an example of how they might be used. We can move out the tutorial description of what the recursion scheme intuitively is, but where should we move it to? Haskell has this horrible problem of assuming so much before a library can be used, and for me this library has been particularly bad for that. It's only years later that I feel comfortable documenting this stuff, but it's really not that hard!

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

So i'd rather move cata extensive documentation to fold (luckily there are common names!), and be rather terse in cata's haddocks.

And let's not give users impression that they need to understand prepro.

@ocharles
Copy link
Author

Ok, happy to move cata, ana and hylo documentation/examples to fold, unfold and refold. I will cross link them so users have a "read more" link when they see cata. I might also add emphasis for Recursive that you don't usually need to worry about the contents of this class, as you can just derive it generically. That will be demonstrated in the module introduction.

And let's not give users impression that they need to understand prepro.

Ok, drop the example and link to the blog post?

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

👍 to introduction. It's not an abstract, doesn't need to be written last (cannot be?)

Really small walk-through should be in the introduction (i.e. first section) of the module. Before documenting any symbols; those are details. We need to highlight what's important, and what's there only for the sake of completeness. In fact, I think there's a lot in this library which is theoretically interesting, but not so useful in practice.

I think SPJ's "how to write a research paper" is spot on also for documenting (a single self-contained) module. Especially the tips about the introduction are spot on:

  1. Describe the problem
  2. State your contributions
    ...and that is all
  • Introduction (1 page, 100 readers)
  • Use an example to introduce the problem
  • Write the list of contributions first
    • The list of contributions drives the entire paper: the paper substantiates the claims you have made

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

Ok, happy to move cata, ana and hylo documentation/examples to fold, unfold and refold

Note, that I'd be happy to reorganise export ordering so these are not last in the docs.

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

And let's not give users impression that they need to understand prepro.
Ok, drop the example and link to the blog post?

Yes. Let's also avoid big multi-line examples if possible (I don't like my cotransverse examples, but had to try it).

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

Also, people will be looking for cataA, transverse and cotraverse, so these fall into "important" group imho. (I have used two first in the real-ish stuff).

@phadej
Copy link
Contributor

phadej commented Jan 30, 2019

Also, we have to mention that recursion-schemes is not complete. Recursive and Corecursive are building blocks. Users might have their own recursion schemes,

e.g. see merge: http://hackage.haskell.org/package/aeson-extra-0.4.1.1/docs/Data-Aeson-Extra-Merge.html it's a specialisation of zipScheme.

zipScheme :: (Recursive a, Corecursive a)
          => (forall x. (x -> x -> x) -> Base a x -> Base a x -> Base a x)
          -> a -> a -> a
zipScheme f = c where c a b = embed $ f c (project a) (project b)

But I cannot find anything like that in recursion-schemes.

@gelisam
Copy link
Collaborator

gelisam commented Jan 30, 2019

Err, @ocharles, since you had replied to my tweet linking to my in-progress documentation, I had assumed you took a look at it and that you wanted to add some extra documentation on top of what I had already written but hadn't yet pushed. But I now see we have duplicated our work by documenting the same things in slightly different ways :(

@gelisam
Copy link
Collaborator

gelisam commented Jan 30, 2019

we have to mention that recursion-schemes is not complete

The README says the following on the topic, is that sufficient?

if you notice that the non-recursive functions you provide themselves seem to share a common pattern, you might be accidentally reimplementing an existing recursion scheme which already has those common parts builtin; or maybe you have stumbled upon a new recursion scheme which does not yet have a name, and which you may want to implement yourself.

@ocharles
Copy link
Author

@gelisam Damn, I wasn't actually aware of that. I only saw the README work, but not module documentation! Where should I be looking?

@gelisam
Copy link
Collaborator

gelisam commented Jan 30, 2019

Btw I do not think "complete" is the right word here, as I do not believe there is a finite set of recursion schemes in the world and that we could attempt to list them all. Rather, there is an infinite number of recursive functions, which share an infinite number of recursion patterns, which we could capture in an infinite number of recursion schemes. These functions, and therefore these recursion schemes, occur with different frequency, and so we have to choose a cut off point below which a scheme which isn't used very often belongs in user code, not this library. I think we are already including recursion schemes which are extremely esoteric, so I'd be in favor of deleting some and adding others, rather than only adding :)

@ocharles
Copy link
Author

Aha, I see you're working in your own branch in your own fork. How about you open a PR here and we can pick whatever you think is "better" in my branch into yours?

@gelisam
Copy link
Collaborator

gelisam commented Jan 30, 2019

Damn, I wasn't actually aware of that. I only saw the README work, but not module documentation! Where should I be looking?

@ocharles look at http://gelisam.com/files/comonadic-recursion-schemes/Data-Functor-Foldable.html, as tweeted here

@ocharles
Copy link
Author

Ok, it looks like you're much further ahead than me, so it's probably best to close this 😅 Don't worry about the duplication of effort from my part, it was a productive exercise either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants