Converters among optional Functions, PartialFunctions and extractor objects #7111

Atry · 2018-08-20T18:16:46Z

This is the implementation for scala/docs.scala-lang#1135

ShaneDelmore · 2018-08-20T23:05:31Z

I find myself converting between these quite frequently, would love to have this out of the box ❤️

Atry · 2018-08-21T13:46:48Z

I updated the proposal to remove the extra extract method call. A PartialFunction is now an extractor.

src/library/scala/Function1.scala

Atry · 2018-08-24T19:25:48Z

Hi @SethTisue , will there be a chance to include this PR in 2.13?

ScalaWilliam · 2018-09-02T18:13:13Z

Very useful. I vote for this being in 2.13.

lrytz · 2018-11-02T15:00:16Z

Looks good (the change) and useful. Seems small enough and self-contained, does it need to go to the SIP meeting? cc @adriaanm

adriaanm · 2018-11-14T12:48:06Z

No, I don't think this needs a SIP

src/library/scala/Predef.scala

sjrd · 2018-11-26T15:35:11Z

This is purely a library-based changed. Even though it touches classes that are typically considered core to the language, I do not believe that this needs to be covered by the SIP process either.

SethTisue · 2018-12-04T21:14:56Z

although we agreed this doesn't need a SIP vote, the writeup you provided at scala/docs.scala-lang#1135 is nonetheless extremely helpful for reviewing this — thank you for that.

I'm in favor of merging this, but I have some questions about specifics:

should Function.unlift be deprecated (and its Scaladoc merged into that for the new method)...?
given that PartialFunction#lift is now synonymous with the new PartialFunction#unapply, should the former be deprecated?
there is an elementWise example in the SIP draft; could this become a Scaladoc @example?
- but also, perhaps it would be better to demonstrate elementWise separately from unlift?
the "cheat sheet" in the SIP draft is cool; is there a natural home for it in the stdlib Scaladoc...?
is elementWise the best possible name? perhaps matchAll?

Atry · 2018-12-05T04:32:00Z

Seth Tisue <[email protected]> 于2018年12月5日周三上午5:15写道：

although we agreed this doesn't need a SIP vote, the writeup you provided at scala/docs.scala-lang#1135 <scala/docs.scala-lang#1135> is nonetheless extremely helpful for reviewing this — thank you for that. I'm in favor of merging this, but I have some questions about specifics: - should Function.unlift be deprecated (and its Scaladoc merged into that for the new method)...?

Function.unlift and UnliftOps.unlift share the same implementation for different use cases, similar to assert and ensuring in Predef. Considering we did not deprecate assert or ensuring, I don't think we have to deprecate Function.unlift.

- given that PartialFunction#lift is now synonymous with the new PartialFunction#unapply, should the former be deprecated?

No, because they are different. unapply is a unary method while lift is a nullary method that returns a unary function.

- there is an elementWise example in the SIP draft; could this become a Scaladoc @example?

Sure

- - but also, perhaps it would be better to demonstrate elementWise separately from unlift?

`elementWise` was originally implemented as `.extract.seq` , which is used for creating extractors for matching nested repeated expressions https://github.com/ThoughtWorksInc/Binding.scala/blob/96a311ec463fb8e47846565c516e23a3fdbc20be/XmlExtractor/src/main/scala/com/thoughtworks/binding/XmlExtractor.scala#L57 https://github.com/ThoughtWorksInc/Binding.scala/blob/96a311ec463fb8e47846565c516e23a3fdbc20be/dom/src/main/scala/com/thoughtworks/binding/dom.scala#L286

- - the "cheat sheet" in the SIP draft is cool; is there a natural home for it in the stdlib Scaladoc...?

Maybe in PartialFunction?

- is elementWise the best possible name? perhaps matchAll?

Given the following code, ``` val firstChar: String => Option[Char] = _.headOption Seq("foo", "bar", "baz") match { case firstChar.unlift.elementWise(c0, c1, c2) => println(s"$c0, $c1, $c2") // Output: f, b, b} ``` you can read it as:

The sequence of foo, bar, baz is matched for the first character on element-wise.

`matchAll` seems a little redundant because there is the keyword match.

…

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7111 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAktumj_ZTDKs5QvqNs8Mq5oMmS_v3Ceks5u1uX3gaJpZM4WEeay> .

-- 杨博 (Yang Bo)

src/library/scala/PartialFunction.scala

ctongfei · 2019-01-06T07:22:53Z

I updated the proposal to remove the extra extract method call. A PartialFunction is now an extractor.

I believe that you should retain the extract name, or something else like invert. In your test case:

  val pf: PartialFunction[Int, String] = {
    case 0 => "zero"
    case 1 => "one"
  }

    1 match {
      case pf(m) =>
        assertEquals(m, "one")
    }

This gives us a feeling that pf("one") == 1;but it is actually the other way around -- it is pf(1) == "one".

I believe that in the clause y match { case f(x) => }, it should be semantically interpreted as "test if y is f(x); if it is, obtain x", or you could say that it is basically solving an equation y = f(x) for x. This is the case for case class deconstruction/unapplying.

Atry · 2019-01-10T13:23:58Z

@ctongfei , it's true that the extractor reversed the result and parameters.

I can rename it back to extract if the SIP committee agrees your opinion.

ctongfei · 2019-01-10T15:11:26Z

@Atry I believe that this is considered a library improvement, so no need for a SIP process?

Atry · 2019-01-10T15:28:55Z

@adriaanm and @SethTisue How do you think of the extract suffix?

adriaanm · 2019-02-04T10:23:48Z

@SethTisue could you look into this one?

adriaanm · 2019-03-18T13:01:00Z

Rebased and squashed.

adriaanm · 2019-03-18T13:02:09Z

@SethTisue it looks like all outstanding review comments were addressed -- could you make a final pass and get this merged?

SethTisue · 2019-03-18T19:15:46Z

@adriaanm and @SethTisue How do you think of the extract suffix?

I think it's fine the way it is. I can see where @ctongfei is coming from, but it seems to me that whenever we have case foo(bar) in any match clause, we always know bar is an output, not an input.

…bjects

SethTisue

let's merge as soon as CI is green. (I just force-pushed a fix for a semantic conflict with #7847)

SethTisue · 2019-03-19T20:36:55Z

the Travis-CI failure is an unrelated Dotty problem (with a fix PR in-flight). green Jenkins is good enough

SethTisue · 2019-03-19T20:37:35Z

@Atry I'm sorry this PR sat so long. it's my fault. thank you for this work, and I'm glad we did finally get it in.

Assigned number 38 to SIP “Converters among optional functions, partial functions, and extractor objects”. Implemented in Scala 2.13.0 and Scala 3.0.0 in scala/scala#7111 The change did not _require_ a SIP because it was implemented at the library-level. Nevertheless, I decided to keep the SIP for posterity.

SethTisue · 2022-06-08T02:43:36Z

@Atry I happened to notice today that this PR is linked from the Scala 2.13.0 release notes, but the PR description lacks any information about what the PR actually adds. Can I interest you in editing the description to add that...?

Assigned number 38 to SIP “Converters among optional functions, partial functions, and extractor objects”. Implemented in Scala 2.13.0 and Scala 3.0.0 in scala/scala#7111 The change did not _require_ a SIP because it was implemented at the library-level. Nevertheless, I decided to keep the SIP for posterity.

scala-jenkins added this to the 2.13.0-RC1 milestone Aug 20, 2018

Atry force-pushed the extractor branch from d72d05c to e514546 Compare August 21, 2018 13:30

Atry force-pushed the extractor branch 2 times, most recently from 2203267 to ea42814 Compare August 21, 2018 15:37

lrytz reviewed Aug 21, 2018

View reviewed changes

src/library/scala/Function1.scala Outdated Show resolved Hide resolved

Atry force-pushed the extractor branch 2 times, most recently from a066d88 to 1f3d18e Compare August 22, 2018 01:01

This comment has been minimized.

Sign in to view

Atry force-pushed the extractor branch from 1f3d18e to 577f336 Compare August 22, 2018 01:24

lrytz assigned adriaanm and SethTisue Nov 2, 2018

martijnhoekstra mentioned this pull request Nov 26, 2018

SIP-NN - Converters among optional Functions, PartialFunctions and extractor objects scala/docs.scala-lang#1135

Merged

sjrd reviewed Nov 26, 2018

View reviewed changes

src/library/scala/Predef.scala Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Atry force-pushed the extractor branch 2 times, most recently from 6ef5897 to d5de017 Compare November 27, 2018 05:26

SethTisue added the release-notes worth highlighting in next release notes label Dec 4, 2018

He-Pin reviewed Dec 15, 2018

View reviewed changes

src/library/scala/PartialFunction.scala Outdated Show resolved Hide resolved

adriaanm removed their assignment Feb 4, 2019

diesalbla added the library:base Changes to the base library, i.e. Function Tuples Try label Mar 17, 2019

adriaanm force-pushed the extractor branch from 3953de5 to 972acab Compare March 18, 2019 13:00

adriaanm requested a review from SethTisue March 18, 2019 13:01

Converters among optional Functions, PartialFunctions and extractor o…

ce891a4

…bjects

SethTisue force-pushed the extractor branch from 972acab to ce891a4 Compare March 18, 2019 19:20

SethTisue approved these changes Mar 18, 2019

View reviewed changes

SethTisue added the prio:blocker release blocker (used only by core team, only near release time) label Mar 19, 2019

SethTisue merged commit 5e547be into scala:2.13.x Mar 19, 2019

Atry deleted the extractor branch March 20, 2019 05:44

Converters among optional Functions, PartialFunctions and extractor objects #7111

Converters among optional Functions, PartialFunctions and extractor objects #7111

Uh oh!

Conversation

Atry commented Aug 20, 2018

Uh oh!

ShaneDelmore commented Aug 20, 2018

Uh oh!

Atry commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Atry commented Aug 24, 2018

Uh oh!

ScalaWilliam commented Sep 2, 2018

Uh oh!

lrytz commented Nov 2, 2018

Uh oh!

adriaanm commented Nov 14, 2018

Uh oh!

Uh oh!

sjrd commented Nov 26, 2018

Uh oh!

This comment has been minimized.

SethTisue commented Dec 4, 2018

Uh oh!

Atry commented Dec 5, 2018 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ctongfei commented Jan 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Atry commented Jan 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctongfei commented Jan 10, 2019

Uh oh!

Atry commented Jan 10, 2019

Uh oh!

adriaanm commented Feb 4, 2019

Uh oh!

adriaanm commented Mar 18, 2019

Uh oh!

adriaanm commented Mar 18, 2019

Uh oh!

SethTisue commented Mar 18, 2019

Uh oh!

SethTisue left a comment

Choose a reason for hiding this comment

Uh oh!

SethTisue commented Mar 19, 2019

Uh oh!

SethTisue commented Mar 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SethTisue commented Jun 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

Atry commented Aug 21, 2018 •

edited

Loading

Atry commented Dec 5, 2018 via email •

edited

Loading

ctongfei commented Jan 6, 2019 •

edited

Loading

Atry commented Jan 10, 2019 •

edited

Loading

SethTisue commented Mar 19, 2019 •

edited

Loading