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

derivations do not make certain methods optional #134

Merged
merged 1 commit into from Jun 15, 2016

Conversation

davidchambers
Copy link
Member

Closes #132

Commit message:

Derivations of FL methods are relevant to implementers of data types.

Derivations of FL methods are irrelevant to implementers of functions which operate on values of these types, since the values must provide the relevant methods.

I'm not sure how toArray and traverse fit into the specification.

@@ -30,9 +30,7 @@ An algebra is a set of values, a set of operators that it is closed
under and some laws it must obey.

Each Fantasy Land algebra is a separate specification. An algebra may
have dependencies on other algebras which must be implemented. An
algebra may also state other algebra methods which do not need to be
implemented and how they can be derived from new methods.
Copy link
Member

Choose a reason for hiding this comment

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

I didn't realize this sentence existed in the spec. That makes most of what I said in #132 questionable.

@rpominov
Copy link
Member

While we are at it, maybe we should add something like:

If a method can be derived and the type also has a hand written version of that method, derived and hand written methods must be equivalent. And if two derived version of a method can be created (for instance we can derive map using ap or chain), they must be equivalent.

To clarify that matter once and for all.

See Gitter discussion: https://gitter.im/fantasyland/fantasy-land?at=570a28a4c65c9a6f7f284c1c

@rpominov
Copy link
Member

I'm not sure how toArray and traverse fit into the specification.

I also don't quite understand why they are here. I think toArray is necessary to define reduce, it restraints how it can be implemented. But traverse seems completely unnecessary.

@davidchambers
Copy link
Member Author

I think toArray is necessary to define reduce, it restraints how it can be implemented.

That may be the case, but the wording can surely be improved. Currently we state:

  1. u.reduce is equivalent to u.toArray().reduce
  • toArray; derivable as function() { return this.reduce((acc, x) => acc.concat([x]), []); }

Could we remove mention of toArray by inlining?

  1. u.reduce is equivalent to u.reduce((acc, x) => acc.concat([x]), []).reduce

@rpominov rpominov mentioned this pull request Apr 13, 2016
@DrBoolean
Copy link

I like that toArray illustrates the power of foldable in a nutshell and gives a law associated with the algebra. I agree that traverse is unnecessary. Perhaps if traversable is rewritten in terms of traverse, it will justify deriving sequence for legacy reasons.

function(m) { return this.chain(f => m.map(f)); }

If a data type provides a method which *could* be derived, its behaviour must
be equivalent to that of the derivation (or derivations).
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 added this sentence based on your suggestion, @rpominov.

Copy link
Member

Choose a reason for hiding this comment

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

Great!

@davidchambers
Copy link
Member Author

The :+1: in #132 (comment) indicates that @puffnfresh is in favour of this change, so let's merge it. :)

I've reverted the removal of toArray based on feedback from @rpominov and @DrBoolean.

@rpominov
Copy link
Member

We should also move reduce derivation that was added after this PR was opened.

@rpominov
Copy link
Member

I've reverted the removal of toArray based on feedback from @rpominov and @DrBoolean.

I like your idea of replacing it with a law:

  1. u.reduce is equivalent to u.reduce((acc, x) => acc.concat([x]), []).reduce

Derivations of FL methods are relevant to implementers of data types.

Derivations of FL methods are irrelevant to implementers of functions
which operate on values of these types, since the values must provide
the relevant methods.
@davidchambers
Copy link
Member Author

Thank you for the feedback, @rpominov. I've made the following changes:

  • removed mention of toArray by inlining;
  • moved the reduce derivation to the Derivations section; and
  • added syntax highlighting to derivations.

@Avaq
Copy link
Contributor

Avaq commented Jun 14, 2016

I prefer this version of the spec over the current. 👍

@puffnfresh
Copy link
Member

👍

@davidchambers
Copy link
Member Author

Thanks, @puffnfresh! Can we merge this now? @SimonRichardson?

@SimonRichardson
Copy link
Member

Sure, do it

On Wed, 15 Jun 2016, 01:34 David Chambers, notifications@github.com wrote:

Thanks, @puffnfresh https://github.com/puffnfresh! Can we merge this
now? @SimonRichardson https://github.com/SimonRichardson?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#134 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ACcaGDe1_p1_iJ-WMaxVehe170-ZfrDCks5qL0iJgaJpZM4IFkwJ
.

@davidchambers
Copy link
Member Author

I'm hoping someone with write access will hit the merge button. ;)

@SimonRichardson SimonRichardson merged commit c7b9d38 into fantasyland:master Jun 15, 2016
@SimonRichardson
Copy link
Member

Haha, done!

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

7 participants