Expand on documentation #217

JordanMartinez · 2020-05-29T01:02:20Z

Highlights include:

Explaining that the Array monad is a nested for loop
Explaining why Monoid and Semigroup have newtypes and why they exist
Added a brief overview of "do notation" and "ado notation" with comprehensive examples
Expand on documentation for Void and distinguish it from C-like languages' void
Provide example of the function <$> arg1 <*> arg2 ... <*> argN pattern

natefaubion · 2020-05-29T01:27:38Z

src/Control/Monad.purs

+-- | Note: if one wants to use "do notation" but redefine what the in-scope
+-- | definitions are for `bind` and `pure` in a given context, one can use
+-- | "qualified do notation." This is an intermediate/advanced language feature
+-- | not explained here.


I'm not sure that Monad is the best place to put documentation for do-notation. There isn't anything requiring a Monad instance in order to use it. This is a common Haskellism, since Haskell requires it, but if you already know to look for that, is this communicating new information?

I thought it might be helpful for new users who are learning FP for the first time.

I understand, and I don't think your intentions are misguided. I'm also not trying to say that the information isn't useful. My point is that core library documentation should strive for correctness. It is not correct to say that do-notation has anything to do with Monad (unlike Applicative-do, which certainly requires Applicative), even if we colloquially tie the two together. It's a common case that when you use do-notation, you likely also have a Monad instance, but this is not a constraint of the language or compiler. If I were a newcomer looking for information on do-notation, it is not clear to me that the Monad typeclass documentation is the best place to put this information. It also is not clear to me that a newcomer is likely to encounter do-syntax for the first time by reading the documentation for Monad.

It's a common case that when you use do-notation, you likely also have a Monad instance, but this is not a constraint of the language or compiler.

Ah... because you can also do what I recall that you do with do (wow... that was a convoluted sentence 😆 ):

foo = do let x = 5 + 2 5 * x

where "do notation" is a somewhat cleaner way of writing "let-in" blocks.

If I were a newcomer looking for information on do-notation, it is not clear to me that the Monad typeclass documentation is the best place to put this information. It also is not clear to me that a newcomer is likely to encounter do-syntax for the first time by reading the documentation for Monad.

Good points! I thought having it here would be nice because one could find something like it in Pursuit when they search for things. However, I agree that there are other forms of documentation (e.g. the book, my repo) that cover it already and might be something new users consult first.

I should also note that what I said was incorrect as well. Applicative-do does not "certainly" require Applicative, since it just inserts calls to apply and pure in scope, even if the common use is with the Applicative class.

I'm fine with removing both of these do/ado notation docs.

I've removed the do/ado notation explanations in latest commits.

JordanMartinez · 2020-06-04T02:08:29Z

I also moved the explanation for Array's "do notation" into the Bind instance due to Harry's comments in the purescript-either PR linked above.

hdgarrood

Thanks for this!

hdgarrood · 2020-09-25T02:08:10Z

src/Control/Bind.purs

@@ -63,6 +63,17 @@ infixr 1 bindFlipped as =<<
 instance bindFn :: Bind ((->) r) where
  bind m f x = f (m x) x

+-- | The `Array` monad's "do notation" works like a nested for loop. Each


I would be more comfortable with an explanation which doesn't refer to do notation here - or, at least, start with one which doesn't refer to do notation, and then add the part about do notation afterwards. That way, at least a part of these docs will make sense to people who don't yet understand the do notation desugaring. I think it's common to use Array's Bind instance explicitly via >>= rather than within a do block as well, which is another reason to understand how Bind Array works outside of the context of do notation.

I think something similar to https://pursuit.purescript.org/packages/purescript-arrays/5.3.1/docs/Data.Array#v:concatMap would be good - although for prelude, if we can avoid referring to downstream libraries in the examples, that would be preferable. We could also add a note to concatMap saying it's the same as bind.

Good point. I've updated that.

hdgarrood · 2020-09-25T02:14:33Z

src/Control/Bind.purs

+-- | foo = do
+-- |   eachElementInArray1 <- [0, 1]
+-- |   eachElementInArray2 <- [1, 2]
+-- |   pure (eachElementInArray1 + eachElementInArray2)


I think this may be clearer if we use a different operation here. How about replacing [0,1] with ["A", "B"] and [1,2] with ["C","D"], and then combining them with <> instead of +? That way each element of the result array is distinct, which means that it's easier to see the order in which the elements are processed.

Agreed! Updated in latest commit.

hdgarrood · 2020-09-25T02:25:57Z

src/Data/Monoid.purs

@@ -28,6 +28,20 @@ import Type.Data.RowList (RLProxy(..))
 -- | `Monoid`s are commonly used as the result of fold operations, where
 -- | `<>` is used to combine individual results, and `mempty` gives the result
 -- | of folding an empty collection of elements.
+-- |
+-- | ### Newtypes for Monoid


I think we should save the specifics of each Monoid-related newtype to the definitions of the newtypes themselves. I suspect it be easier for people to understand these if they can see the rendered definitions alongside the documentation which says how they work, and also this would be very easy to get out of sync since the newtypes themselves live in separate files.

I do think this is a good place to mention the issue that some types permit multiple Monoid instances, and that we can express this with newtypes, but I think it would be best to keep that discussion minimal here. Something like "For example, the Additive newtype has a Monoid instance which is based on Semiring's plus and zero, and the Multiplicative newtype has a Monoid instance which is based on Semiring's mul and one. There are a number of similar newtypes in the submodules of Data.Monoid in this package.

Also, I realise this is a bit of a nitpick, but I would also prefer not to call it a "workaround", because I think doing this means admitting that not being able to equip a type with two Monoid instances is a deficiency (I don't think it is).

I would also prefer not to call it a "workaround"

Agreed.

Does my rendering work? Or are there other changes you would prefer?

This looks great 👍

hdgarrood · 2020-09-25T02:26:31Z

src/Data/Monoid.purs

+-- |
+-- | Some types (e.g. `Int`, `Boolean`) can implement multiple law-abiding
+-- | instances for `Monoid`. For example, `<>` could be `+` and `mempty` could
+-- | be `0`. Likewise, `<>` could be `*` and `mempty` could be `1`.


I think "Alternatively" or "Equally" might make more sense than "Likewise" here?

hdgarrood · 2020-09-25T02:29:36Z

src/Data/Semigroup.purs

+-- | There are two other ways to implement an instance for this type class
+-- | regardless of which type is used. These instances can be used by
+-- | wrapping the values in one of the two newtypes below:
+-- | 1. `First` - Use the first argument every time: `append first _ = first`.


These instances are a bit weird and somewhat rarely used, so I'm not sure they deserve a mention here.

I would say they deserve a mention because it mirrors what is said in Monoid. It's better for a reader to read this, then read the Data.Semigroup.First documentation, and then think, "I get what Semigroup and First are, and First is not something I need. However, I know where to go if I do need it later" rather than "I get what a Semigroup is" and only later come across First and have to infer its relation.

I just think that of all the things we might want to say about semigroups to someone encountering the concept for the first time, I think the First and Last semigroups should be fairly low down on the list, even if their newtypes do happen to live in this repository. While having some level of understanding of the Semigroup type class is more or less required if you want to use PureScript, I would argue that the First and Last semigroups are very much not required. I think it’s more respectful of the user’s time and energy to avoid mentioning things if we don’t expect that they will find them useful at the moment they are reading the docs in question.

Then again, they are simple examples, and giving plenty of simple examples is always an important part of teaching these concepts, so maybe they do deserve a mention here.

hdgarrood · 2020-09-25T02:34:38Z

src/Data/Void.purs

 -- |
 -- | `Void` is useful to eliminate the possibility of a value being created.
 -- | For example, a value of type `Either Void Boolean` can never have
 -- | a Left value created in PureScript.
+-- |
+-- | This should not be confused with the word, `void,` that commonly appears in
+-- | C-deriving languages, such as Java:


nit: I think "C-derived" would be more grammatically appropriate than "C-deriving" here. Or you could use "C-family"? Apparently that's more common now: https://books.google.com/ngrams/graph?corpus=26&smoothing=3&year_start=1800&year_end=2019&content=C-derived+languages%2CC-family+languages&direct_url=t1%3B%2CC%20-%20derived%20languages%3B%2Cc0%3B.t1%3B%2CC%20-%20family%20languages%3B%2Cc0

I used C-family

hdgarrood · 2020-09-25T02:36:44Z

src/Data/Void.purs

 -- |
 -- | `Void` is useful to eliminate the possibility of a value being created.
 -- | For example, a value of type `Either Void Boolean` can never have
 -- | a Left value created in PureScript.
+-- |
+-- | This should not be confused with the word, `void,` that commonly appears in


nit: the comma after "word" shouldn't really be there, and also the comma inside the backticks is a bit awkward. How about "This should not be confused with the keyword void that commonly appears in..."?

Sounds good. See latest commits.

hdgarrood · 2020-09-25T02:38:09Z

src/Data/Void.purs

+-- | ```
+-- |
+-- | In PureScript, one often uses `Unit` to achieve similar effects as
+-- | the lowercased `void` above.


I think it would be slightly clearer to say "the void of C-family languages" here, so that the reader doesn't have to double-check the casing to see which void is which in order to know which one is meant here.

Good point. Done

hdgarrood · 2020-09-25T02:38:36Z

src/Data/Semigroup.purs

@@ -18,7 +18,16 @@ import Type.Data.RowList (RLProxy(..))
 -- | - Associativity: `(x <> y) <> z = x <> (y <> z)`
 -- |
 -- | One example of a `Semigroup` is `String`, with `(<>)` defined as string
-- | concatenation.
+-- | concatenation. Another example is `List a`, with `(<>)` defined as


This is definitely a good example to include here 👍

hdgarrood · 2020-09-25T23:15:10Z

src/Control/Bind.purs

+-- | `bind`/`>>=` adds another level of nesting in the loop:
+-- | ```
+-- | foo :: Array Int
+-- | foo = do


Best to remove this do with this change, I think

hdgarrood · 2020-09-25T23:15:54Z

src/Control/Bind.purs

+-- |     ["c", "d"] >>= \eachElementInArray2
+-- |       pure (eachElementInArray1 <> eachElementInArray2)
+-- |
+-- | foo == [ ("a" <> "c"), ("a" <> "d"), ("b" <> "c"), ("b" <> "d") == [ "ac", "ad", "bc", "bd"]


I think at this point the [ ("a" <> "c"), ... ] is probably just adding noise - part of the reason I suggested changing the operation in this example is so that it could be done away with and we could just write foo == [ "ac", "ad", "bc", "bd" ]. Also, I think it would be good to have the spaces either side of the [] match - at the moment there is a space after the [ but there is not a space before the ].

hdgarrood · 2020-09-25T23:18:38Z

src/Control/Bind.purs

@@ -63,6 +63,17 @@ infixr 1 bindFlipped as =<<
 instance bindFn :: Bind ((->) r) where
  bind m f x = f (m x) x

+-- | The `Array` monad works like a nested for loop. Each


Sorry, just to clarify what I was trying to get at before: I think it's best to provide a precise explanation which says what the thing does before trying to build intuition. In this case I think we should follow the lead of the docs in Data.Array.concatMap to start with before mentioning the similarity to a nested for loop. Perhaps:

The bind/>>= function for Array works by applying a function to each element in the array, and flattening the results into a single, new array. Array's bind works like a nested for loop; each bind adds another level of nesting in the loop:

Makes sense. I'll use that rendering.

hdgarrood

Thanks for being patient with my many change suggestions :)

JordanMartinez · 2020-09-26T22:10:19Z

Thanks for being patient with my many change suggestions :)

No worries!

JordanMartinez · 2020-09-26T22:12:20Z

The build is failing because of the v0.14.0-compatibility changes I made a while back.

hdgarrood · 2020-09-26T22:33:21Z

I'd like to get #225 merged next as that should fix CI.

JordanMartinez added 11 commits May 28, 2020 12:54

Indicate that Array monad is a nested for loop

9464e86

Expand on Monoid's documentation to explain purpose behind newtypes

276a5e5

Add another example of (<>): list concatenation

c48a6e6

Expan on Semigroup's documentation to explain purposes behind newtypes

e7de566

Expand on 'uninhabitated data type' meaning for Void

da12208

Distinguish Void from lowercase void in C-deriving languages

7f4e169

Add example of f <$> arg1 <*> arg2 <*> ... <*> argN to Apply docs

8157482

Add quick overview of do notation in Monad docs

285f934

Add quick overview for ado notation in Applicative

0b62cf3

Decrease header level to 3 for Monad and Applicative "ado/do notation"

6587f19

Add level 3 header for overview of Monoid/Semigroup newtypes

bf1a5b4

natefaubion reviewed May 29, 2020

View reviewed changes

JordanMartinez mentioned this pull request Jun 4, 2020

Show Either's "do notation" similarity to nested if-then-else statement purescript/purescript-either#52

Merged

JordanMartinez added 3 commits June 3, 2020 19:04

Remove do/ado notation explanation in Monad and Applicative

0fb3e86

Port Array's "do notation" explanation from Monad to Bind instance

0bfaad0

Clarify what causes nesting in Array's Bind instance explanation

ec6710f

hdgarrood reviewed Sep 25, 2020

View reviewed changes

Do not use do notation for explaining Array's Bind instance.

2224347

hdgarrood reviewed Sep 25, 2020

View reviewed changes

JordanMartinez added 9 commits September 25, 2020 17:40

Rework the Monoid explanation when referring to its newtypes

5ab4e52

Use "C-family" rather than "C-deriving" rendering

07bef30

Refer to void as a keyword and fix comma mistake

8d79a6b

Use 'void of C-family languages' rendering

24f4ca7

Remove unneeded do in Array's Bind explanation

d939da5

Provide a simple graph reduction of examples in Array's Bind explanation

b424064

Fix spacing between closing ]

a608415

Explain concept precisely and then use nested for loop intuition

105a53e

Change Array's element type to String to match example

85039e8

hdgarrood approved these changes Sep 26, 2020

View reviewed changes

Merge branch 'master' into addDocs

6e14a53

hdgarrood merged commit 85aa2db into purescript:master Sep 27, 2020

JordanMartinez deleted the addDocs branch September 24, 2021 14:37

Expand on documentation #217

Expand on documentation #217

Uh oh!

Conversation

JordanMartinez commented May 29, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

natefaubion May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanMartinez May 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanMartinez commented Jun 4, 2020

Uh oh!

hdgarrood left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JordanMartinez Sep 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

natefaubion May 29, 2020 •

edited

Loading

JordanMartinez May 29, 2020 •

edited

Loading

JordanMartinez Sep 26, 2020 •

edited

Loading