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

fantasy land: add Fantasy Land functions #216

Merged
merged 1 commit into from
Feb 12, 2017
Merged

Conversation

davidchambers
Copy link
Member

@davidchambers davidchambers commented May 11, 2016

Closes #178; closes #252

This pull request adds the following functions:

  • alt :: Alt f => f a -> f a -> f a
  • ap :: Apply f => f (a -> b) -> f a -> f b (ap replaces S)
  • apFirst :: Apply f => f a -> f b -> f a
  • apSecond :: Apply f => f a -> f b -> f b
  • bimap :: Bifunctor f => (a -> b) -> (c -> d) -> f a c -> f b d
  • chain :: Chain m => (a -> m b) -> m a -> m b
  • chainRec :: ChainRec m => TypeRep m -> (a -> Either (m b) (m b)) -> a -> m b
  • empty :: Monoid a => TypeRep a -> a
  • equals :: Setoid a => a -> a -> Boolean
  • extend :: Extend w => (w a -> b) -> w a -> w b
  • extract :: Comonad w => w a -> a
  • filter :: (Applicative f, Foldable f, Monoid (f a)) => (a -> Boolean) -> f a -> f a
  • filterM :: (Monad m, Monoid (m a)) => (a -> Boolean) -> m a -> m a
  • join :: Chain m => m (m a) -> m a
  • map :: Functor f => (a -> b) -> f a -> f b (map replaces B and lift)
  • of :: Applicative f => TypeRep f -> a -> f a
  • promap :: Profunctor p => (a -> b) -> (c -> d) -> p b c -> p a d
  • sequence :: (Applicative f, Traversable t) => TypeRep f -> t (f a) -> f (t a)
  • toString :: Any -> String
  • traverse :: (Applicative f, Traversable t) => TypeRep f -> (a -> f b) -> t a -> f (t b)
  • zero :: Plus f => TypeRep f -> f a

Original description:

sanctuary-js/sanctuary-type-classes#2 hasn't yet generated the excitement I expected, so I thought I'd open a pull request to add all the Fantasy Land functions except equals and of while removing code.

🌈

@davidchambers davidchambers mentioned this pull request May 11, 2016
@svozza
Copy link
Member

svozza commented May 11, 2016

Ah so this is where old map functions go to die. ;) Looks great. I wasn't sure how to handle those test failures in S.match either, I presume it's because of the weird object/array that you get back from using the native regex function in JS?

index.js Outdated
}

}(function(R, $) {
}(function(R, λ, $) {
Copy link
Member

@svozza svozza May 11, 2016

Choose a reason for hiding this comment

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

What's the quickest way to type this symbol without having to resort to copy and paste?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure. I placed it in the l register in Vim so I was able to type " l p to insert it.

We should probably switch to an identifier that's easier to type. _, perhaps? I like one-character identifiers for dependencies we reference dozens of times. :)

Copy link
Member

Choose a reason for hiding this comment

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

L for lambda?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, L sounds good.

@davidchambers
Copy link
Member Author

I wasn't sure how to handle those test failures in S.match either, I presume it's because of the weird object/array that you get back from using the native regex function in JS?

The problem actually lies with optional non-capturing groups. Since there's no type of which both 'foo' and undefined are members, ['foo', undefined] is heterogeneous and therefore considered invalid.

S.map(S.I, /(foo)?(bar)?/.exec('foobar'));
// => ['foobar', 'foo', 'bar']

S.map(S.I, /(foo)?(bar)?/.exec('foo'));
// ! TypeError: Type-variable constraint violation
//
//   map :: (Functor a, Functor b) => Function -> a -> b
//                                                ^
//                                                1
//
//   1)  ["foo", "foo", undefined, "index": 0, "input": "foo"] :: Array ???
//
//   Since there is no type of which all the above values are members, the type-variable constraint has been violated.

The problematic expression is map(toMaybe, match). The solution is to replace this with a for loop (match.map(toMaybe) wouldn't work since toMaybe would complain about receiving three arguments).
The fact that we must work around the type system when bringing inconsistent values into the consistent world doesn't worry me – in fact, it's a sign that the type system is doing its job. :)

@svozza
Copy link
Member

svozza commented May 11, 2016

The fact that we must work around the type system when bringing inconsistent values into the consistent world doesn't worry me – in fact, it's a sign that the type system is doing its job. :)

I agree completely, it's better to deal with these sort of oddities in the safety of a function's scope rather than having them leak out, even if it's at the cost of the implementation's elegance.

@davidchambers davidchambers mentioned this pull request May 16, 2016
@davidchambers davidchambers force-pushed the dc-fantasy-land branch 3 times, most recently from 61547af to 10f9831 Compare May 18, 2016 01:36
@davidchambers davidchambers force-pushed the dc-fantasy-land branch 2 times, most recently from 32436ce to 1f11c7d Compare July 4, 2016 07:19
@davidchambers davidchambers force-pushed the dc-fantasy-land branch 2 times, most recently from ce4f395 to 082d153 Compare September 14, 2016 02:12
This was referenced Sep 14, 2016
@davidchambers davidchambers force-pushed the dc-fantasy-land branch 7 times, most recently from a0d31c9 to 2b807c0 Compare October 17, 2016 07:31
//. > S.bimap(S.toUpper, Math.sqrt, S.Right(64))
//. Right(8)
//. ```
S.bimap =
Copy link
Member

Choose a reason for hiding this comment

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

Does that mean we don't need S.either anymore?

Copy link
Member

Choose a reason for hiding this comment

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

@svozza S.either is for unpacking an Either a b, it returns the return value of the mapper. S.bimap returns a new Either of the value returned by the mapper.

Copy link
Member

Choose a reason for hiding this comment

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

Oh yes, of course, either is similar to fromMaybe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, although it's most similar to maybe and maybe_:

maybe  ::        b  -> (a -> b) -> Maybe a -> b
maybe_ :: (() -> b) -> (a -> b) -> Maybe a -> b
either ::  (a -> c) -> (b -> c) -> Either a b -> c

fromMaybe and fromEither are also closely related:

fromMaybe  :: a -> Maybe a -> a
fromEither :: b -> Either a b -> b

Copy link
Member

@safareli safareli left a comment

Choose a reason for hiding this comment

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

Laws looks good, but chainRec signature is not correct.

index.js Outdated
//. > S.equals(S.Just([1, 2, 3]), S.Just(['1', '2', '3']))
//. false
//. ```
S.equals = def('equals', {}, [a, b, $.Boolean], Z.equals);
Copy link
Member

Choose a reason for hiding this comment

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

Returns true if its arguments are of the same type and equal according to the type's fantasy-land/equals method; false otherwise.

Shouldn't it be (Setoid a) => a -> a -> Boolean instead of a -> b -> Boolean ?

Copy link
Member Author

Choose a reason for hiding this comment

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

That depends on whether we think S.equals('foo', 42) should evaluate to false or throw a type error. I'm in favour of the former, since the latter would make S.equals cumbersome to use in test suites. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Returning false is more practical indeed. What if we add equalsStrict or something which has a -> a -> bool and throws type error? but it could be done in other PR, after we are we really want it, personally I think it would be good, as it will prevent some type errors in programs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see value in providing a second function. We might want to name it something bland like equals_, though, to avoid confusion with strict (===) equality.

Copy link
Member

Choose a reason for hiding this comment

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

nice ⚡️

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added equals_ :: Setoid a => a -> a -> Boolean. I'm now wondering whether equals and equals_ should be switched. The _ suffix is used elsewhere in the library as a subtle warning to users, and I believe we should discourage users from comparing values of different types. Do you agree?

Copy link
Member

@safareli safareli Jan 21, 2017

Choose a reason for hiding this comment

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

I think in "your" codebase where you know types of values, you will only use equals when two values have same type, as if you know that one value has different type then the other, it's useless to use equals as it will return false for every value of those types.

But one might use equals in library like code, when someone outside of the library could pass some value of any type. But for cases like this one could just use S.unchecked.equals.

So after writing/thinking on it i think equals:: a -> a -> Boolean would be nice if one could also do this S.unchecked.equals to access unchecked version of equals

Copy link
Member Author

Choose a reason for hiding this comment

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

So after writing/thinking on it i think equals:: a -> a -> Boolean would be nice if one could also do this S.unchecked.equals to access unchecked version of equals

Great idea! Using the checkTypes switch to avoid having two versions of the same function is very elegant. I've pushed a commit to replace S.equals with the stricter variant.

In #262 Aldwin and I have been talking about bringing back S.unchecked, which would allow S.unchecked.equals as you suggest.

//. Nothing
//. ```
S.zero =
def('zero', {f: [Z.Plus]}, [TypeRep($.TypeVariable('f')), f(a)], Z.zero);
Copy link
Member

Choose a reason for hiding this comment

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

why is it needed to have TypeRep($.TypeVariable('f')) instead of TypeRep(f)?

Copy link
Member Author

Choose a reason for hiding this comment

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

f :: Type -> Type is not a member of sanctuary-def's internal Type type. Should it be? I don't know. For now, at least, we need to provide a value of type Type.

Copy link
Member

Choose a reason for hiding this comment

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

not sure, just looked strange

index.js Outdated
//. Nothing
//. ```
function traverse(typeRep, f, traversable) {
function pure(x) { return Z.of(typeRep, x); }
Copy link
Member

Choose a reason for hiding this comment

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

I think this part (and sequence) should be updated once sanctuary-js/sanctuary-type-classes#28 is merged

index.js Outdated
//. ```
S.join = def('join', {m: [Z.Chain]}, [m(m(a)), m(a)], Z.join);

//# chainRec :: ChainRec m => TypeRep m -> (a -> Either (m b) (m b)) -> a -> m b
Copy link
Member

Choose a reason for hiding this comment

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

Full implementation of chainRec (I think it should be correct, but isn't tested)

def('chainRec',
  {m: [Z.ChainRec]},
  [TypeRep($.TypeVariable('m')), Fn(a, m($Either(a, b))), a, m(b)],
  // :: ChainRec m => TypeRep m -> (a -> m (Either a b)) -> a -> m b
  function chainRec(typeRep, f, x) {
    // :: (a -> c, b -> c) -> Either a b -> c
    function mapper(next, done) {
      // :: Either a b -> c
      return function mapper$inner(either) {
        return either.fold(next, done)
      }
    }
    // :: (a -> c, b -> c, a) -> m c
    function step(next, done, x) {
      // :: m Either a b
      var mEither = f(x)
      // :: Either a b -> c
      var g = mapper(next,done)
      return Z.map(g, mEither)
    }
    return Z.chainRec(typeRep, step, x)
  })
)

Also Would be nice to note in comments, which part of Either represent Done and which Loop.

//. ```
S.filter =
def('filter',
{f: [Z.Applicative, Z.Foldable, Z.Monoid]},
Copy link
Member

Choose a reason for hiding this comment

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

(f a) should be a Monoid not f, (same for Monoid (m a) in filterM), but looks like there is no other way right?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a limitation of sanctuary-def. I'd love to work out how to properly capture this constraint.

};

ComposeFG.prototype[FL.ap] = function(other) {
return ComposeFG(Z.ap(Z.map(function(u) { return function(y) { return Z.ap(u, y); }; }, other.value), this.value));
return ComposeFG(Z.ap(Z.map(S.ap, other.value), this.value));
Copy link
Member

Choose a reason for hiding this comment

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

Is the reason to mix Z and S the fact that Z has uncurried functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right. Perhaps it would be better to use our internal ap function instead (in both cases). What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think that would a good approach, as when we move this laws in a separate module, we might not want it to be depend on sanctuary, because it has curried functions .

Copy link
Member Author

Choose a reason for hiding this comment

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

test/sequence.js Outdated
eq(typeof S.sequence, 'function');
eq(S.sequence.length, 2);
eq(S.sequence.toString(), 'sequence :: (Applicative f, Traversable t) => TypeRep f -> t (f b) -> f (t b)');

Copy link
Member

Choose a reason for hiding this comment

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

TK?

Copy link
Member Author

Choose a reason for hiding this comment

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

eq(typeof S.promap, 'function');
eq(S.promap.length, 3);
eq(S.promap.toString(), 'promap :: Profunctor p => (a -> b) -> (c -> d) -> p b c -> p a d');

Copy link
Member

Choose a reason for hiding this comment

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

TK?

Copy link
Member Author

Choose a reason for hiding this comment

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

eq(typeof S.of, 'function');
eq(S.of.length, 2);
eq(S.of.toString(), 'of :: Applicative f => TypeRep f -> a -> f a');

Copy link
Member

Choose a reason for hiding this comment

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

TK?

Copy link
Member Author

Choose a reason for hiding this comment

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

test/join.js Outdated
eq(S.join([[[1, 2, 3]]]), [[1, 2, 3]]);
eq(S.join(S.Just(S.Just(1))), S.Just(1));
eq(S.join(S.Just(S.Just(S.Just(1)))), S.Just(S.Just(1)));

Copy link
Member

Choose a reason for hiding this comment

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

maybe also add failing case?

+  eq(S.join(S.Nothing)), S.Nothing);

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidchambers davidchambers force-pushed the dc-fantasy-land branch 2 times, most recently from 46a83e5 to 1488ac7 Compare January 21, 2017 15:13
Copy link
Member

@safareli safareli left a comment

Choose a reason for hiding this comment

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

As chainRec is fixed, this LGTM. only adding of some tests is left which is not a big issue ✌️

index.js Outdated
//. [[1, 2, 3]]
//.
//. > S.join(S.concat)('abc')
//. 'abcabc'
Copy link
Member Author

Choose a reason for hiding this comment

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

Yesterday when I asked about \x -> f x x @brendanhay told me that join is the W combinator! Very cool! See Stack Overflow if you're interested in the substitutions which show that S.join(f) is equivalent to x => f(x)(x).

Copy link
Member

@Avaq Avaq Feb 4, 2017

Choose a reason for hiding this comment

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

I was just looking at it. Quite something. So we have:

map = B
ap = S
join = W

I'll add them to my table. ;)

@Avaq Avaq mentioned this pull request Feb 4, 2017
3 tasks
index.js Outdated
//. Function a (b -> c) -> Function a b -> Function a c
//. (a -> b -> c) -> (a -> b) -> (a -> c)
//.
//. This is the S combinator from combinatory logic.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe move the conclusion above the explanation, so a reader knows why he's reading it, and may choose beforehand to skim over it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. "This" in the sentence above refers to (a -> b -> c) -> (a -> b) -> (a -> c) rather than to ap, which is more general. We could, though, combine this sentence with the sentence preceding the substitutions:

Replacing Apply f => f with Function x produces the S combinator from combinatory logic:

Copy link
Member Author

Choose a reason for hiding this comment

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

index.js Outdated
//. (b -> c) -> Function a b -> Function a c
//. (b -> c) -> (a -> b) -> (a -> c)
//.
//. [`compose`](#compose) is thus a specialized form of `map`.
Copy link
Member

Choose a reason for hiding this comment

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

Same remark as with ap

index.js Outdated
//. Function a (Function a b) -> Function a b
//. (a -> a -> b) -> (a -> b)
//.
//. This is the W combinator from combinatory logic.
Copy link
Member

Choose a reason for hiding this comment

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

Same remark as with ap.

@davidchambers davidchambers force-pushed the dc-fantasy-land branch 2 times, most recently from ca13bf2 to 982f175 Compare February 12, 2017 16:36
test/chainRec.js Outdated
: map(S.Left)(function(env) { return n + env.step; });
}

eq(S.chainRec(Function, stepper, 0)({step: 2, inc: 100}), 3100);
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm correct that this test is as much to examine stack behaviour as output, perhaps that warrants a comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea.

Copy link
Member

@Avaq Avaq left a comment

Choose a reason for hiding this comment

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

I'm really excited about this!

@@ -0,0 +1,28 @@
'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

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

It strikes me that it would be nice to test these typeclass functions on as minimal a case as possible i.e. the FreeFoo.

Certainly out of scope for this PR, and I'm not sure where you get your functors from, but what do you think about it in principle?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you suggesting defining a type which provides only the methods and/or TypeRep functions necessary to satisfy the requirements of the type class?

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty much. I was thinking we could use the free structures (free monoids, applicatives, monads ...). You do need a base functor for those, though, and I'm not sure whether the natural choice (Identity) actually gives you much assurance.

index.js Outdated
//.
//. Chain m => m (m a) -> m a
//. Function x (Function x a) -> Function x a
//. Function x (Function x b) -> Function x b
Copy link
Contributor

@rjmk rjmk Feb 12, 2017

Choose a reason for hiding this comment

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

I don't know how much the variable renaming helps understanding here

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you suggesting this instead?

Chain m => m (m a) -> m a
Function x (Function x a) -> Function x a
(x -> x -> a) -> (x -> a)

I'm happy to make this change.

Are you suggesting removing the alpha-renaming from the map and ap documentation as well, or were you commenting on join specifically?

I find the alpha-renaming useful for map and ap as I find the signatures more recognizable when the conventional identifiers are used. Also, it's a good idea to perform alpha-renaming one variable at a time if one is reusing identifiers. Otherwise variables may appear to move, which is not the correct interpretation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Only chain really stuck out to me, but perhaps it's better to be consistent

Copy link
Member Author

Choose a reason for hiding this comment

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

Consistency led me to the longer version, but I like your shorter version. When there are only two type variables I'm happy to deal with x and a. :)

Copy link
Contributor

@rjmk rjmk left a comment

Choose a reason for hiding this comment

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

Awesome!

@davidchambers davidchambers force-pushed the dc-fantasy-land branch 3 times, most recently from 2c5feff to 18792e1 Compare February 12, 2017 20:05
@davidchambers
Copy link
Member Author

🦄

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.

mention Fantasy Land in readme (pointfree) chain: Chain f => (a -> f a) -> f a -> f a
7 participants