everything #3

gabejohnson · 2017-06-08T04:27:13Z

Closes #1

safareli · 2017-06-08T07:40:59Z

index.js

+  //. Unit
+  //. ```
+  var Unit = {};
+  Unit.constructor = Object.create(Unit.constructor);


Unit.constructor on the right side is just Object, is there some different intention?

An artifact of an earlier implementation. I'll change it.

safareli · 2017-06-08T07:42:33Z

index.js

+    return Z.lte(this.a, other.a) && Z.lte(this.b, other.b);
+  };
+
+  function makePair(p, a, b) {


that's the point of this function

Because Identity inherits prototypically from Pair it's necessary to check the type of p when creating a new one in Pair methods. Pair being the ancestor shouldn't really have to worry about this, so if you have another solution I'd love to hear it.

makes sense

In my view the minimal code sharing afforded by implementing Identity a in terms of Pair Unit a is insufficient compensation for the complexity of inheritance. I'd prefer to avoid inheritance, as demonstrated in cd8a8c5.

Also if someone adds some method to prototype of Pear it will be accessible on Identity too which might not be desired

Code sharing wasn't the point. I noticed that fixing the first argument to Pair to be Unit automatically resulted in Identity. It had been my intention to (potentially) follow PureScript in having a single tuple type (Pair) and implement tuple3, tuple4, etc. in terms of it.

But whereas PureScript's tuple3(1, 2, 3) would give Pair(1, Pair(2, Pair(3, Unit))) mine would give Pair(Pair(Pair(Unit, 1), 2), 3) to allow map, ap, etc. to work over the final argument. Consistency FTW.

That said, I'm happy to distinguish tuple1 from Identity.

safareli · 2017-06-08T07:45:22Z

index.js

+  //. > Z.toString(Pair(1, 2))
+  //. 'Pair(1, 2)'
+  //. ```
+  Pair.prototype.inspect =


related fluture-js/Fluture#68

Makes sense. I'll change it.

gabejohnson · 2017-06-08T12:38:39Z

Thanks for looking it over @safareli!

davidchambers · 2017-06-09T05:45:04Z

README.md

@@ -0,0 +1,351 @@
+# sanctuary-tuples


⚠️ Let's remember to remove this file before merging this pull request. :)

Wait. Remove the README?

Yes. README.md is generated as part of the release process. Note that in

https://github.com/sanctuary-js/sanctuary-tuples/blob/v/index.js#L54

there is no version number, so these links are currently broken. When we run make release-major, though, VERSION will be set to 1.0.0 so the links will point to the v1.0.0 tag. :)

davidchambers · 2017-06-09T05:49:01Z

bower.json

+    "/Makefile",
+    "/circle.yml",
+    "/package.json"
+  ]


Let's have a white list ("files") rather than a blacklist ("ignore").

davidchambers · 2017-06-09T06:36:57Z

index.js

+//. [Monad]:            v:fantasyland/fantasy-land#monad
+//. [Ord]:              v:fantasyland/fantasy-land#ord
+//. [Semigroupoid]:     v:fantasyland/fantasy-land#semigroupoid
+//. [Semigroup]:        v:fantasyland/fantasy-land#semigroup


Nit: Semigroup should come before Semigroupoid.

davidchambers · 2017-06-09T06:38:09Z

test/index.js

+
+test('lte', function() {
+  eq(Z.lte(Unit, Unit), true);
+  eq(Z.lte(Unit, Object.create(Unit)), false);


I think we should remove this test case.

These should actually use Object.assign which isn't available on all of the platforms tested. It's purpose is to demonstrate that both arguments must be identical to Unit.

davidchambers · 2017-06-09T06:43:23Z

index.js

+  //.
+  //. ```javascript
+  //. > Z.ap(Pair(' there', Math.sqrt), Pair('hello', 64))
+  //. Pair('hello there', 8)


Haskell disagrees on the order in which the a values should be concatenated:

> (<*>) (" there", sqrt) ("hello", 64) (" therehello", 8.0)

I think this has to do with the argument reordering FL does WRT ap. Though I could be wrong.

Haskell:

$ ghci > :t (<*>) (<*>) :: Applicative f => f (a -> b) -> f a -> f b

Fantasy Land:

$ node > const S = require('sanctuary') > S.ap ap :: Apply f => f (a -> b) -> f a -> f b

The type signatures are in agreement.

I was referring to the change in FL < 1.0 vs > 1.0
pre 1.0

ap :: Apply f => f (a -> b) ~> f a -> f b

post 1.0

ap :: Apply f => f a ~> f (a -> b) -> f b

But I suppose we can implement it however we want as long as the signatures match.

And being consistent w/ Haskell is probably a safe bet.

davidchambers · 2017-06-09T06:49:53Z

test/index.js

+var Pair = Tuples.Pair;
+var Identity = Tuples.Identity;
+var fst = Tuples.fst;
+var snd = Tuples.snd;


The presence of fst and snd suggest to me that we would be better off having separate packages for Unit, Identity, and Pair. We would then have Pair.fst and Pair.snd, which would be much clearer. This would also prevent implementation details from leaking:

> Tuples.snd(Tuples.Identity(42)) 42

we would be better off having separate packages for Unit, Identity, and Pair.

I can get behind that. Though the Unit package would be quite small (no pun intended) and sanctuary-tuples would have a dependency on it.

Would you still want functions fst and snd? Or just have the user rely on property access?

I'm very much in favour of keeping the functions. As with isNothing and isJust, it's nice to provide both property and function to support various use cases.

gabejohnson · 2017-06-12T17:12:07Z

@davidchambers how do I set up CircleCI for this repo?

davidchambers · 2017-06-12T17:32:41Z

how do I set up CircleCI for this repo?

I just visited https://circleci.com/gh/sanctuary-js/sanctuary-tuples and followed the project. I believe that's all that's required.

gabejohnson · 2017-06-13T15:09:20Z

@davidchambers aside from removing README.md and changing the ASCII art, I think this is ready.

gabejohnson · 2017-07-12T15:31:58Z

@davidchambers do you think this is good to go (minus the ASCII art)?

davidchambers · 2017-07-12T15:40:13Z

I would very much like to incorporate fantasyland/fantasy-laws#1. I intend to complete and merge that pull request this weekend. In the interim, you could add fantasy-laws to package.json as a Git dependency and use it to test the lawfulness of this algebraic data type. :)

davidchambers · 2017-07-24T09:16:49Z

@gabejohnson, what do you think of the idea of exporting Pair, Pair.fst, Pair.snd, … rather than P.Pair, P.fst, P.snd, …?

gabejohnson · 2017-07-24T12:18:08Z

I think that makes sense. The current export method is a holdover from a previous implementation when we were exporting more types.

davidchambers · 2017-08-04T09:54:23Z

We should define Pair.chainRec. As usual, I think I'll need help from you, @safareli. ;)

gabejohnson · 2017-08-04T18:25:39Z

Makefile

@@ -45,7 +45,6 @@ lint:
 	  --global module \
 	  --global require \
 	  --global self \
-	  --rule 'max-len: [off]' \


@davidchambers oops! I thought the line length violation was from the link to MDN. Thanks for fixing it 😄

The linting rules are actually pretty sophisticated. We ignore lines containing URLs, and we provide a pattern to match //# lines as we don't want to wrap type signatures.

You were just one character over the limit, by the way. ;)

davidchambers · 2017-08-21T10:02:12Z

index.js

+  //. ```
+  function Pair$prototype$lte(other) {
+    if (Z.equals(this.fst, other.fst)) return Z.lte(this.snd, other.snd);
+    return Z.lte(this.fst, other.fst);


This form makes the symmetry a little clearer:

return Z.equals(this.fst, other.fst) ? Z.lte(this.snd, other.snd) : Z.lte(this.fst, other.fst);

davidchambers · 2017-08-21T13:19:56Z

index.js

+           /      /   /
+          /______/   /
+          \      \  /
+           \______\/                */


How about this?

/* *\ // \\ // @@ @@ @@ @@ \\ // @@ @@ @@ \\ \\ @@ @@ @@ // \\ @@ @@ @ @@ @ // \\ / @ // \* @@@@ */

Lookin' good David!

davidchambers · 2018-05-10T11:20:41Z

index.js

+  //. . 'Extend          ✅   ',
+  //. . 'Comonad         ✅   ',
+  //. . 'Contravariant   ❌   ' ]
+  //. ```


@gabejohnson, please review this table. I don't think it's possible to satisfy ChainRec because we'd be forced to produce a value of type Pair a d without access to a value of type a.

Sorry it took me so long to respond to this David. I'm not certain what a ChainRec instance would look like if we did implement it. But given the way it's defined in the spec, I don't see how we could anyway. We would have to provide a TypeRep for Semigroup a => Pair a.

If we develop some new technique in the future which allows for this, we can always revisit 😄

Sorry it took me so long to respond to this David.

No worries. Your timing is very good. I will soon merge this pull request. :)

I would like to use the Co-authored-by trailer so the final (squashed) commit will be jointly attributed. Are you happy for me to include your email address in the commit message? If not, I will use gabejohnson@users.noreply.github.com instead.

Using my email address is fine.

The release of these days type libraries is very exciting 😀

davidchambers · 2018-05-28T11:25:21Z

I'm about to squash the commits. The original commits can be found at 838e48e.

Co-authored-by: Gabe Johnson <gijohnson105@gmail.com> Co-authored-by: Matt Willhite <matthew.willhite@gmail.com>

davidchambers · 2018-05-28T11:38:38Z

Thank you for your significant contributions, @gabejohnson and @miwillhite! 🍻

miwillhite · 2018-05-29T02:15:57Z

Woohoo, great work all!

gabejohnson requested a review from davidchambers June 8, 2017 04:27

safareli reviewed Jun 8, 2017

View reviewed changes

gabejohnson force-pushed the gabejohnson/everything branch 3 times, most recently from 61a86c7 to 21fc14f Compare June 8, 2017 13:14

davidchambers reviewed Jun 9, 2017

View reviewed changes

gabejohnson force-pushed the gabejohnson/everything branch from 1b2397c to 8e54831 Compare June 13, 2017 15:06

gabejohnson force-pushed the gabejohnson/everything branch from 6745c81 to 99eebbb Compare July 12, 2017 15:34

gabejohnson commented Aug 4, 2017

View reviewed changes

davidchambers reviewed Aug 21, 2017

View reviewed changes

davidchambers mentioned this pull request Aug 21, 2017

Testing against fantasy-laws #4

Merged

12 tasks

davidchambers reviewed Aug 21, 2017

View reviewed changes

davidchambers mentioned this pull request Jan 14, 2018

array: add S.partition sanctuary-js/sanctuary#386

Closed

davidchambers mentioned this pull request Feb 13, 2018

❤️ ❤️ ❤️ sanctuary-js/sanctuary#487

Open

davidchambers reviewed May 10, 2018

View reviewed changes

davidchambers mentioned this pull request May 10, 2018

everything sanctuary-js/sanctuary-identity#3

Merged

1 task

davidchambers force-pushed the gabejohnson/everything branch from 838e48e to ff8f401 Compare May 28, 2018 11:30

everything

90744d5

Co-authored-by: Gabe Johnson <gijohnson105@gmail.com> Co-authored-by: Matt Willhite <matthew.willhite@gmail.com>

davidchambers force-pushed the gabejohnson/everything branch from ff8f401 to 90744d5 Compare May 28, 2018 11:34

davidchambers merged commit 824bbda into master May 28, 2018

davidchambers deleted the gabejohnson/everything branch May 28, 2018 11:37

This pull request was closed.

everything #3

everything #3

Conversation

gabejohnson commented Jun 8, 2017

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

gabejohnson Jun 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabejohnson commented Jun 8, 2017

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

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

gabejohnson commented Jun 12, 2017

davidchambers commented Jun 12, 2017

gabejohnson commented Jun 13, 2017

gabejohnson commented Jul 12, 2017

davidchambers commented Jul 12, 2017

davidchambers commented Jul 24, 2017

gabejohnson commented Jul 24, 2017 • edited Loading

davidchambers commented Aug 4, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidchambers commented May 28, 2018

davidchambers commented May 28, 2018

miwillhite commented May 29, 2018

gabejohnson Jun 9, 2017 •

edited

Loading

gabejohnson commented Jul 24, 2017 •

edited

Loading