PartialFunction.run shadowing scala.sys.process.ProcessBuilder.run

106 views
Skip to first unread message

Vlad Ureche

unread,
May 7, 2012, 9:23:05 AM5/7/12
to scala-i...@googlegroups.com, pavel.e...@gmail.com
Hi everyone,

I noticed there's a shadowing problem involving PartialFunction.run and scala.sys.process.ProcessBuilder.run:

scala.PartialFunction defines run[U](x: Int)(action: (A) => U): Boolean
scala.collection.Seq inherits run from PartialFunction
scala.sys.process.ProcessImplicits defines an implicit conversion from Seq[String] to ProcessBuilder
to start the process of a ProcessBuilder one needs to call the .run() method => which is shadowed by the run method in PartialFunction

Can we rename the run method in PartialFunction?

Thanks,
Vlad

martin odersky

unread,
May 7, 2012, 9:46:02 AM5/7/12
to scala-i...@googlegroups.com
What does it do anyway? It does not have a ScalaDoc comment.

Thanks

 - Martin
 

Pavel Pavlov

unread,
May 7, 2012, 9:52:58 AM5/7/12
to scala-i...@googlegroups.com, pavel.e...@gmail.com
Hi Vlad and all.

I've added `run` & `runWith` - two imperative-style combinators to PartialFunction 3 months ago:
https://groups.google.com/forum/#!msg/scala-internals/mtMDGik1i1c/jzAostbxW8IJ

The whole point of all this was to solve old performance-related as well as double-matcher evaluation problems of partial functions.

However, as I've not received any feedback on this, it's not clear for me how these two methods should be named nor should them be added to public PartialFunction interface at all.

So any feedback on this is appreciated.

Ragerds,
Pavel.

Pavel Pavlov

unread,
May 7, 2012, 10:06:27 AM5/7/12
to scala-i...@googlegroups.com
Hi Martin,

Sorry for not finishing it yet.

These two methods (run & runWith) was intended for fast partial function application in some widespread scenarios, such as method `collect` in scala collections:

  def collect[B, That](pf: PartialFunction[A, B])(implicit bf: CanBuildFrom[Repr, B, That]): That = {
    val b = bf(repr)
    val action = pf runWith { y => b += y }
    this foreach action
    b.result
  }

See this post for details:
https://groups.google.com/forum/#!msg/scala-internals/mtMDGik1i1c/jzAostbxW8IJ

It was my proposed extension to PartialFunction interface. However, I did not received much feedback on this, so I really don't have any clue whether such extension is desirable and/or acceptable. Maybe you could refine on this?

As regards to missing ScalaDocs, I'll fix them in a week if these methods will survive at all.

Regards,
Pavel.


понедельник, 7 мая 2012 г., 20:46:02 UTC+7 пользователь martin написал:

martin odersky

unread,
May 7, 2012, 10:13:13 AM5/7/12
to scala-i...@googlegroups.com
On Mon, May 7, 2012 at 4:06 PM, Pavel Pavlov <pavel.e...@gmail.com> wrote:
Hi Martin,

Sorry for not finishing it yet.

These two methods (run & runWith) was intended for fast partial function application in some widespread scenarios, such as method `collect` in scala collections:

  def collect[B, That](pf: PartialFunction[A, B])(implicit bf: CanBuildFrom[Repr, B, That]): That = {
    val b = bf(repr)
    val action = pf runWith { y => b += y }
    this foreach action
    b.result
  }

See this post for details:
https://groups.google.com/forum/#!msg/scala-internals/mtMDGik1i1c/jzAostbxW8IJ

It was my proposed extension to PartialFunction interface. However, I did not received much feedback on this, so I really don't have any clue whether such extension is desirable and/or acceptable. Maybe you could refine on this?

As regards to missing ScalaDocs, I'll fix them in a week if these methods will survive at all.

Hi Pavel,

We need someone else to lead the spec of this. I am too swamped with reflection to give this much thought. Looking at your original message, it was not clear to me how runWith differs from andThen and I did not see a usage example of run. So I am still mystified what they are?

I agree that if we keep them we need more specific names for them.

Cheers

 - Martin

Adriaan Moors

unread,
May 7, 2012, 11:12:47 AM5/7/12
to scala-i...@googlegroups.com
Hi Pavel,

Unless these methods are actually used, I suggest we drop them. This thread is another example of the burden of adding stuff[*] to interfaces.

When we do get around to optimizing collect, and it turns out andThen does not suffice, we can always restore them.
When we do, I agree with Martin that the names run/runWith are too vague, so we should find a more descriptive name.

thank you again for your contribution, and thank you for your understanding regarding the limited amount of feedback
we do try our best, and we really appreciate your contributions --  there's simply a lot going on at the same time

chers

Pavel Pavlov

unread,
May 7, 2012, 11:18:20 AM5/7/12
to scala-i...@googlegroups.com
Martin,

I've found that very common use of partial functions can be expressed as a pattern:
  if (pf.isDefinedAt(x)) someAction(pf(x))

or slightly different variant of the same pattern:
  if (pf.isDefinedAt(x)) { someAction(pf(x)); true } else false

Such code cannot be rewritten with andThen (as (pf andThen someAction)(x)) without double evaluation of pattern matchers/conditions for PF literals (one in `isDefinedAt`, another in `apply`) as andThen will throw MatchError if pf is not defined at `x`. The cause of this is the same as of slow `orElse` issue which you've fixed once in Nov/Dec 2011.

So, `pf.run(x)(someAction)` and `pf.runWith(someAction)(x)` are the combinators which implement exactly the behavior of the code patterns above without double evaluation of pf's matchers/conditions and thus they are potentially more effective in performance-aware scenarios.
One of such scenarios I've found in the standard Scala collections library is the method `collect`. The current version needs to evaluate pf's matchers twice for each collection element that is not filtered out. Besides potential performance issues and double side-effects [1] this can lead to getting out of sync between pattern matchers and actions in PF literals [2]. Methods `run` & `runWith` solve all these problems.

[1] http://stackoverflow.com/questions/4064859/is-the-partialfunction-design-inefficient
[2] https://groups.google.com/forum/#!msg/scala-internals/sbvCLxPyDcA/6dr40vqUS40J

Regards,
Pavel.



понедельник, 7 мая 2012 г., 21:13:13 UTC+7 пользователь martin написал:

Adriaan Moors

unread,
May 7, 2012, 11:30:26 AM5/7/12
to scala-i...@googlegroups.com
but if (pf.isDefinedAt(x)) someAction(pf(x)) is captured by (pf andThen someAction)(x), right?
that's what collect needs, as far as I can tell

the boolean version feels a bit too specialized to me, and I think it corresponds to
(pf andThen {x => someAction(x); true}).applyOrElse(x, false)

Pavel Pavlov

unread,
May 7, 2012, 11:48:29 AM5/7/12
to scala-i...@googlegroups.com

2012/5/7 Adriaan Moors <adriaa...@epfl.ch>

but if (pf.isDefinedAt(x)) someAction(pf(x)) is captured by (pf andThen someAction)(x), right?
that's what collect needs, as far as I can tell


Not quite so:

Welcome to Scala version 2.10.0-M3 (Java HotSpot(TM) Client VM, Java 1.6.0_16).
Type in expressions to have them evaluated.
Type :help for more information.

scala> :paste
// Entering paste mode (ctrl-D to finish)

def foo(x: Any, pf: PartialFunction[Any, Any]) { if(pf.isDefinedAt(x)) println(pf(x)) }
def bar(x: Any, pf: PartialFunction[Any, Any]) { pf.run(x)(println) }
def baz(x: Any, pf: PartialFunction[Any, Any]) { (pf andThen println)(x) }

// Exiting paste mode, now interpreting.

foo: (x: Any, pf: PartialFunction[Any,Any])Unit
bar: (x: Any, pf: PartialFunction[Any,Any])Unit
baz: (x: Any, pf: PartialFunction[Any,Any])Unit

scala> for(x <- Seq(1,2)) foo(x, { case 1 => 11 })
11

scala> for(x <- Seq(1,2)) bar(x, { case 1 => 11 })
11

scala> for(x <- Seq(1,2)) baz(x, { case 1 => 11 })
11
scala.MatchError: 2 (of class java.lang.Integer)
        at scala.PartialFunction$$anon$2.apply(PartialFunction.scala:188)


Adriaan Moors

unread,
May 7, 2012, 12:05:22 PM5/7/12
to scala-i...@googlegroups.com
scala> def noop(x: Any) {}
noop: (x: Any)Unit

scala> def baz(x: Any, pf: PartialFunction[Any, Any]) { (pf andThen println).applyOrElse(x, noop) }
baz: (x: Any, pf: PartialFunction[Any,Any])Unit

scala> for(x <- Seq(1,2)) baz(x, { case 1 => 11 })
11

√iktor Ҡlang

unread,
May 14, 2012, 5:53:02 AM5/14/12
to scala-i...@googlegroups.com
Can we please make applyOrElse @inline?
--
Viktor Klang

Akka Tech Lead
Typesafe - The software stack for applications that scale

Twitter: @viktorklang

Adriaan Moors

unread,
May 14, 2012, 5:54:41 AM5/14/12
to scala-i...@googlegroups.com
yes you can!

√iktor Ҡlang

unread,
May 14, 2012, 6:07:57 AM5/14/12
to scala-i...@googlegroups.com


On Mon, May 14, 2012 at 11:54 AM, Adriaan Moors <adriaa...@gmail.com> wrote:
yes you can!

badass

Som Snytt

unread,
May 21, 2012, 4:21:17 PM5/21/12
to scala-i...@googlegroups.com
From the groundlings, thanks for the interesting code and thread.

Even if every call to p initiates a new nuclear war,

  if (p(x)) f(x)

should either happen or not happen.  There is no room in the land of
sane code for a third choice of "or we throw an exception if p changes
its mind when we double check."

Though I'm still reconciling that with:

Lots of things, most things even, may or may not throw an exception.

Everything is, at bottom, Nothing.  Or not everything.  At least not the thing behind everything.  From a mortal's perspective.  I guess that would be a blue pill perspective.


Reply all
Reply to author
Forward
0 new messages