Skip to content

add optional to Applicative - #1560

Closed
julien-truffaut wants to merge 1 commit into
masterfrom
optional
Closed

add optional to Applicative#1560
julien-truffaut wants to merge 1 commit into
masterfrom
optional

Conversation

@julien-truffaut

Copy link
Copy Markdown
Contributor

No description provided.

@kailuowang

kailuowang commented Mar 15, 2017

Copy link
Copy Markdown
Contributor

Edit: duh, I didn't see the pure , never mind my question. but I agree with @johnynek

I am curious can this in be in Functor?

def sequence[G[_], A](as: G[F[A]])(implicit G: Traverse[G]): F[G[A]] =
G.sequence(as)(this)

/** one or none */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need more comments than this. For F = List this will be one and none (we will always concatenate None to the list), so it behavior seems highly dependant on the F.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the intended usecase of this method?

@codecov-io

codecov-io commented Mar 15, 2017

Copy link
Copy Markdown

Codecov Report

Merging #1560 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1560      +/-   ##
==========================================
+ Coverage   92.49%   92.49%   +<.01%     
==========================================
  Files         246      246              
  Lines        3891     3892       +1     
  Branches      136      135       -1     
==========================================
+ Hits         3599     3600       +1     
  Misses        292      292
Impacted Files Coverage Δ
core/src/main/scala/cats/Applicative.scala 70% <100%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 954b4e2...aae2e76. Read the comment docs.

@julien-truffaut

Copy link
Copy Markdown
Contributor Author

@kailuowang we need pure from Applicative, I don't think it can be a Functor

@johnynek I thought about the same just after submitting the PR, as you say the behaviour depends mainly on F. Do you have a suggestion for another doc?

@kailuowang kailuowang added this to the 1.0.0-MF milestone Apr 11, 2017
@kailuowang kailuowang mentioned this pull request May 25, 2017
26 tasks
@kailuowang

Copy link
Copy Markdown
Contributor

@julien-truffaut do you need this for the next release?

@julien-truffaut

julien-truffaut commented Jul 13, 2017 via email

Copy link
Copy Markdown
Contributor Author

@kailuowang

Copy link
Copy Markdown
Contributor

Closing stale PRs. Feel free to reopen if there is interest to revive the effort.

@kailuowang kailuowang closed this Aug 5, 2019
@johnynek

johnynek commented Aug 5, 2019

Copy link
Copy Markdown
Contributor

just a note, isn't Applicative + SemigroupK == Alternative? shouldn't this be on alternative?

@LukaJCB

LukaJCB commented Aug 5, 2019

Copy link
Copy Markdown
Member

@johnynek Applicative + MonoidK == Alternative, not SemigroupK alone

@larsrh
larsrh deleted the optional branch April 11, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants