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

Adds Identity API to docs #38

Merged
merged 9 commits into from
Feb 10, 2017
Merged

Adds Identity API to docs #38

merged 9 commits into from
Feb 10, 2017

Conversation

rstegg
Copy link
Contributor

@rstegg rstegg commented Jan 26, 2017

This is a PR for issue #42.

A WIP API for crocks; you should review it and change what you'd like, and add to readme; it's modeled after the most.js api

Not much to dispute here; mostly copied from readme 🧀

@evilsoft
Copy link
Owner

This is an amazing start!
Thanks so much. Would you mind adding this folder to the .npmignore so we do not push documentation to the package.

I am a little worried about this getting stale and an extra thing to remember to do. It could get out of date easily. So we may want to update the main readme to lose the hard coupling.

But that is something we can fix up.

@evilsoft
Copy link
Owner

Don't worry about updating the README, we can adjust that after the docs get a bit more fleshed out.

docs/API.md Outdated

## Monoids

Each `Monoid` provides a means to represent a binary operation and is usually locked down to a specific type. These are great when you need to combine a list of values down to one value. In this library, any ADT that provides both an `empty` and `concat` function can be used as a `Monoid`. There are a few of the `Crocks` that are also monoidial, so be on the look out for those as well. All `Monoids` work with the point-free functions `mconcat`, `mreduce`, `mconcatMap` and `mreduceMap`.
Copy link
Owner

Choose a reason for hiding this comment

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

Why get rid of the crocks description and leave this one?

Copy link
Owner

Choose a reason for hiding this comment

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

Actually the more I think about it, we should probably keep all descriptions here.
So instead of removing the descriptions, we should probably add the crocks description back.

docs/API.md Outdated

## Point-free

While it can seem natural to work with all these containers in a fluent fashion, it can get cumbersome and hard to get a lot of reuse out of. A way to really get the most out of reusability in Javascript is to take what is called a point-free approach. Below is a small code same to contrast the difference between the two calling styles:
Copy link
Owner

Choose a reason for hiding this comment

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

Also may want to get rid of this one as well?

Copy link
Owner

Choose a reason for hiding this comment

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

Keep this and add back the crocks description.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was not sure whether to add them here or keep them in the README, so they were purposely inconsistent for option; does your new issue #42 makes these suggestions obsolete, or what do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

I would add the crocks section. We can take it out if we need to once we cut over to these docs.

Add docs to npmignore
Copy link
Contributor Author

@rstegg rstegg left a comment

Choose a reason for hiding this comment

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

Pointing out differences in the types of definitions. The ideas are there but the examples [looking at you, Constructor of] might suck

### chain

`(a -> m b) -> m b`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different types of instance definitions: explicit Function vs implicit f, and implicit m vs (not sure what m stands for)

Copy link
Owner

Choose a reason for hiding this comment

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

Here m is used to represent any Monad:
Monad m => (a -> m b) -> m b.

So as this is specifically an Identity, it could (and maybe should) be written as:
(a -> Identity b) -> Identity b

Although it would probably look better as:
Identity m => (a -> m b) -> m b

If that makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then should all defs be prefixed by the representing monad?

crocks.Identity.of([1,2,10])
crocks.Identity([1,2,10])
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Different type of definition for constructor; here is only a definition by example

Copy link
Owner

Choose a reason for hiding this comment

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

of is a very specific function that signals an Applicative.
In many cases the normal constructor function is the same thing as of.
We still include of in those cases so any functions that rely on calling of can call it.

So the constructor parameters should be represented by the type siggy in most cases.
For things like Writer it takes a Monoid and kicks you back a constructor that is fixed to that Monoid like: Writer (Sum b) a

Also for Maybe, it also takes an internal UnionType and if it is not the UnionType, it delegates to Maybe.of (which is basically Maybe.Just)

Good 👁 tho.

Copy link
Owner

@evilsoft evilsoft Jan 29, 2017

Choose a reason for hiding this comment

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

Actually a better example to highlight the difference would be IO.
You can only construct an IO with a function. BUT of will take anything you pass it and not check it at all and give you back an IO, that when run, returns whatever it was you passed of

@evilsoft
Copy link
Owner

@rstegg This is really shaping up nicely.
Thanks so much for jumping into this so enthusiastically. Excited to see what you do with all of this.

@rstegg rstegg changed the base branch from master to docs February 6, 2017 20:22
@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 077e492 on rstegg:navigation-api into 5745f6b on evilsoft:docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 177289e on rstegg:navigation-api into 5745f6b on evilsoft:docs.

1 similar comment
@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 177289e on rstegg:navigation-api into 5745f6b on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ebc65ea on rstegg:navigation-api into 5745f6b on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 1d0c505 on rstegg:navigation-api into 5745f6b on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling bed1526 on rstegg:navigation-api into 5745f6b on evilsoft:docs.

const dinner = frozenPizza.map(microwave) //Identity "microwave pizza"
dinner.value() // "microwave pizza"
```
To expose the constructor and instances defined below
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To expose the constructor and instances defined below to the argument ?


### equals

`x -> Boolean`
`Identity x => x -> Boolean`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❓ : does this really need to be defined here or should a top-level api define this?

@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 121433d on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling fa420c7 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

Copy link
Owner

@evilsoft evilsoft left a comment

Choose a reason for hiding this comment

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

Please check out my comments, and please ask any questions if you have them!!


### of

`Identity a => a -> Identity a`
Copy link
Owner

Choose a reason for hiding this comment

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

Should be something like:
Identity m => a -> m a


## Instances

### inspect
Copy link
Owner

Choose a reason for hiding this comment

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

inspect should be along the lines of:
() -> String

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Identity () -> ?

Copy link
Owner

Choose a reason for hiding this comment

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

No need to call out Identity here. this function just maps unit to a string.


### value

`() -> a`
Copy link
Owner

Choose a reason for hiding this comment

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

value should show be all like:

`Identity m => m a ~> () => a

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not understand this one, folding Identity m => m a -> a?

Copy link
Owner

Choose a reason for hiding this comment

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

value just maps unit to whatever is inside the identity.

// `a` would be Number in this case
Identity(3)
  .value()  // returns 3


### type

`() -> Identity`
Copy link
Owner

Choose a reason for hiding this comment

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

type returns a string identifier so something like:
() -> String


### map

`Identity m => m (a -> b) -> m b`
Copy link
Owner

Choose a reason for hiding this comment

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

So map takes a function and lifts it into the context of Identity, and because we need to show how the mapping works, we will want to use ~> here. So this is how it should be shown IMO:
Identity m => m a ~> (a -> b) -> m b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sweet!


### ap

`Identity m => m a ~> m (a -> b) -> m b`
Copy link
Owner

Choose a reason for hiding this comment

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

This siggy matches the Fantasy Land spec and it just so happens this is the only place in crocks that does not match the spec (for reasons). So to get it to show how crocks handles Apply, it should be something like:
Identity m => m (a -> b) ~> m a -> m b.

Notice how it is reversed.

Copy link
Contributor Author

@rstegg rstegg Feb 7, 2017

Choose a reason for hiding this comment

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

why do i feel like half of this can be thrown away?

Identity m => m (a -> b) or Identity m => m a -> m b? (just factoring the m, not actually defining a signature, is it?)

Copy link
Owner

Choose a reason for hiding this comment

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

I don't know what you feel that.

There is a constraint that the instance that this is being called on, MUST contain a function.
So that is why m (a -> b) is needed to the left of ~>...We need to communicate that constraint.

Make sense? Do you have any ideas on how we could better communicate that constraint?
👂

Copy link
Contributor Author

@rstegg rstegg Feb 7, 2017

Choose a reason for hiding this comment

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

Identity m => m a -> (a -> b) ~> m a -> m b

Copy link
Contributor Author

@rstegg rstegg Feb 7, 2017

Choose a reason for hiding this comment

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

well i feel silly for trying now


### of

`Identity a => a -> Identity a`
Copy link
Owner

Choose a reason for hiding this comment

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

Should stick with the "shorthand" for 🌽sistancy. Even though this is on the instance for 🌽venience, we do not really need to show the instance, as this function is not dependent on the instance (?)

Identity m => a -> m a


### chain

`Identity m => m a ~> (a -> m b) -> m b`
Copy link
Owner

Choose a reason for hiding this comment

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

🐱fect!! This is exactly right!!


### sequence

`Identity m, Applicative f => m (f b) ~> f (m a) -> m (f a)`
Copy link
Owner

Choose a reason for hiding this comment

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

So for sequence we need an f returning function as the argument, but it is independent of the other types in this flow. so we can show that by using b like this:
Identity m, Applicative f => m (f a) ~> (b -> f b) -> f (m a)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Curious if m a ~> (b -> f b) -> f (m a) works for 🌽 sistency to traverse then?

Copy link
Owner

Choose a reason for hiding this comment

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

nope. we need to already contain the f a for sequence.

In fact, that is the thing about sequence, it must already contain the Applicative.
Where traverse has a value and takes a function that will lift that value (and apply any additional mapping to it, if it wants) into the Applicative, and then sequence over that.

Does that make sense?

Copy link
Owner

Choose a reason for hiding this comment

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

Good 👁 though! in catching the similarity


### traverse

`Identity m, Applicative f => m a ~> f (m b) -> m (f b)`
Copy link
Owner

Choose a reason for hiding this comment

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

And this bad boy should be something along the lines of
Identity m, Applicative f => m a ~> (c -> f c) -> (a -> f b) -> f (m b)

(The siggy is wrong for the crocks pointfree function. I will update that with the next PR. It should take a m a and not a m (f a) like it says. 🙄 )

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b2775be on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

2 similar comments
@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling b2775be on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b2775be on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 82df8f7 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 7, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 3020043 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8389a03 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8389a03 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

1 similar comment
@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 8389a03 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@rstegg
Copy link
Contributor Author

rstegg commented Feb 8, 2017

On Either, only worked on the sigs, nothing above it. next commit after review will have those changes

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 3736fbb on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@coveralls
Copy link

coveralls commented Feb 8, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 32be547 on rstegg:navigation-api into 7a4ba53 on evilsoft:docs.

@rstegg rstegg changed the title Add docs/API.md documentation, no changes to Readme Add Identity API to docs Feb 10, 2017
@rstegg rstegg changed the title Add Identity API to docs Adds Identity API to docs Feb 10, 2017
@evilsoft
Copy link
Owner

Merging...Here is a squirrel

@evilsoft evilsoft merged commit 6674ae9 into evilsoft:docs Feb 10, 2017
evilsoft pushed a commit that referenced this pull request Mar 1, 2017
* Add API documentation
evilsoft pushed a commit that referenced this pull request Mar 13, 2017
* Add API documentation
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

3 participants