Add Map operations.

[julienrf]

No description provided.

[julienrf]

It’s a bit unfortunate that we have to implement all these methods.

[julienrf]

Note that filterKeys and mapValues are not anymore lazy.

BTW, it’s good to see that their implementation is simpler than in the current collections!

[Ichoran]

We have to rename them, then. A difference in behavior like that can silently break code in unpleasant ways. It's annoying that they were always lazy, but we have to allow the user an opportunity to have every single spot pointed out to them. (We can auto-rewrite the ones that were obviously lazy-version-then-immediately-something-eager.)

[julienrf]

Otherwise, if we implement a lazy semantics what do you think of changing their return type to View[(K, V)] instead of CC[K, V]?

[Ichoran]

Switching it to a view isn't good either because it no longer has map semantics. You could switch it to a map view if we had such a thing.

[julienrf]

You can do .to(Map) to get back Map semantics. Or did you mean something else by “map semantics”?

[Ichoran]

No, that doesn't work because it's not lazy any more. The output of filterKeys and mapValues is, for better or worse, a lazy map. Lazy-but-not-a-map is not a map. Map-but-not-lazy is not lazy. We can't know what properties people have relied upon in their code, so the only reasonable approach is to deprecate or remove the entire method and introduce another that does something that we think is less confusing, or maintain both the laziness and the map-ness.

[julienrf]

I think returning a View is fine, if people rely on Map-specific methods then they’ll get a compile error. I think the fact that filterKeys and mapValues return a “lazy Map” in the current collections can be a source of confusion and we should not support that in the strawman. Moreover, in the strawman we have a cleaner distinction between lazy and strict collections so we should take advantage of that.

[Ichoran]

So we're taking away the functionality completely, without a deprecation and no way to recover it, and not even changing the method name? Maybe it will be okay because nobody's using them for what they can do, but otherwise this is precisely the kind of thing that tends to upset users (unannounced breaking change with no workaround).

[julienrf]

Dotty reports a type error here:

def removeAll(keys: IterableOnce[K]): C = fromSpecificIterable(keys.iterator().foldLeft(coll)(_ - _))
                                                                                              ^^^^^
                                                    found:    C
                                                    required: CC[K, V]

But I think he is wrong.
Indeed, the type of coll is CC[K, V], which is upper bounded by MapOps[K, V, CC, CC[K, V]] (note that the last parameter (whose formal name is C) is instantiated to CC[K, V]). Then, the - operator returns a C, which is a CC[K, V].

cc @odersky.

[julienrf]

In this commit I try an alternative solution but I still get an error (different, though), with Dotty:

def removeAll(keys: IterableOnce[K]): C = fromSpecificIterable(keys.iterator().foldLeft[CC[K, V]](coll)(_ - _))
                                                                                                        ^^^
                                 value `-` is not a member of CC[K, V]

Which, again, seems wrong to me: CC is upper bounded by MapOps, which provides the - method.

[julienrf]

I’ll leave this PR for later because it has several compilation errors with dotty.

[Dveim]

What are reasons to have lazy mapValues method?

Current implementations don't distinguish view and don't rehash keys functionalities, what probably should not go as default. There are several cases where strict implementation (mapValuesNow) would have advantage. Why don't make it default behaviour?

[Ichoran]

@Dveim - The reason is to allow the strawman to function as a drop-in replacement for the existing collections. This has been a primary goal all along. Since the behavior changes radically when it is lazy vs. eager, simply swapping one for another isn't something you can do blind, even if it compiles. A lazy mapValues could be used to access a single element, making the eager version a million times slower. Alternatively, the lazy version could load a file every time on something that's accessed many times, making the lazy version ten thousand times slower. These kinds of speed differences are not just a performance detail, but the difference between working and effectively broken programs. So I don't think we can change the behavior without changing the method name so people at least have a chance to think about what's going on.

[Dveim]

Thanks for answer. Indeed, compatibility alone is enough to leave things as is. Do you plan to add strict version, `mapValuesNow` ?

[amasson88]

see also https://issues.scala-lang.org/browse/SI-4776

[julienrf]

If we keep the lazy behavior of mapValues, then IMHO its static return type should be View rather than Map (and we should probably have a MapView[K, V] type of view with Map-like operations on it).

Then, do we want a strict version? We can always write xs.mapValues(f).to(HashMap) to force the evaluation.

Also, I didn’t carefully check but I think in my projects my main usage of mapValues is to transform the values of a groupBy result. However, I think we will have another variant of groupBy that will combine groupBy and (a strict version of) mapValues (see #42). So, if that corresponds to the main usage of this method for the majority of the community too, then I don’t think we need another a strict variant of mapValues.

[julienrf]

I rebased to master and bumped the dotty version to take advantage of scala/scala3#2889 but we now have other compilation errors:

[error] -- [E045] Syntax Error: /home/julien/workspace/dev/scala/collection-strawman/src/test/scala/strawman/collection/test/GenericTest.scala:54:38 
[error] 54 |    Parse.parseCollection[(Int, Int), immutable.HashMap[Int, Int]](immutable.HashMap).parse("1-2|3-4").contains(immutable.HashMap((1, 2), (3, 4)))
[error]    |                                      ^^^^^^^^^
[error]    |                                      cyclic reference involving type CC
[error] -- [E008] Member Not Found Error: /home/julien/workspace/dev/scala/collection-strawman/src/test/scala/strawman/collection/test/Test.scala:498:47 
[error] 498 |    val xs7 = (xs: immutable.Map[String, Int]) map ({ case (k, v) => (new Foo, v) }: scala.PartialFunction[(String, Int), (Foo, Int)])
[error]     |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |value `map` is not a member of strawman.collection.immutable.Map[java.lang.String, scala.Int]

[julienrf]

Dotty fix in progress: scala/scala3#2914 😄