-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[#10919] Deprecate Map.{filterKeys,mapValues} while undeprecating those in MapView #7014
Conversation
1252cbf
to
9a76300
Compare
A number of tests now issue deprecation warnings (https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/3621/), they need to be updated to use |
@@ -193,13 +193,15 @@ trait MapOps[K, +V, +CC[_, _] <: IterableOps[_, AnyConstr, _], +C] | |||
* @return an immutable map consisting only of those key value pairs of this map where the key satisfies | |||
* the predicate `p`. The resulting map wraps the original map without copying any elements. | |||
*/ | |||
@deprecated("Use view.filterKeys. A future version will include a strict version of this method.", "2.13.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say:
Use
.view.filterKeys(p)
.
(The second sentence looks good)
We could maybe also document how to perform this operation in a strict way (.view.filterKeys(p).toMap
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julienrf I'm on it
…se in MapView [10919] Remove added scaladocs [10919] Replace usages of Map#mapValues with Map#view.mapValues [10919] Reduction of expression in SymbolTables, replace usage of Map#mapValues with Map#view.mapValues [10919] Remove usages of Map#filterKeys [10919] Fix broken filterKeys and mapValues in concurrent.TrieMap
1bac1de
to
03ab559
Compare
@julienrf this is ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome @joshlemer, thanks!
else super.mapValues(f) | ||
} | ||
|
||
override def view: MapView[K, V] = if (nonReadOnly) readOnlySnapshot().view else super.view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All thanks too the existing exhaustive tests :-)
def filterKeys(p: K => Boolean): MapView[K, V] = new MapView.FilterKeys(this, p) | ||
|
||
/** Transforms this map by applying a function to every retrieved value. | ||
* @param f the function used to transform values of this map. | ||
* @return a map view which maps every key of this map | ||
* to `f(this(key))`. The resulting map wraps the original map without copying any elements. | ||
*/ | ||
@deprecated("Use .view.mapValues(f). A future version will include a strict version of this method (for now, .view.mapValues(f).toMap).", "2.13.0") | ||
def mapValues[W](f: V => W): MapView[K, W] = new MapView.MapValues(this, f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should change the implementations of these to .view.mapValues(f)
and .view.filterKeys(p)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that’s fine as it is.
thank you Josh! |
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12. Issues addressed with this upgrade: ### Underscore (`_`) is no longer a valid identifer for vals Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name: ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12. Issues addressed with this upgrade: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12 & 2.11. Issues addressed with this upgrade: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12 & 2.11. Issues addressed with this upgrade: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12 & 2.11. Issues addressed with this upgrade: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12 & 2.11. Although Scala 2.13.1 has been released, this change sticks with 2.13.0, as `scoverage` has not yet released an update compatible with 2.13.1: scoverage/sbt-scoverage#295 scoverage/scalac-scoverage-plugin#283 scoverage/sbt-scoverage#299 Migration issues addressed with the update to Scala 2.13: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This project now compiles with Scala 2.13 by default, while still cross-compiling to Scala 2.12 & 2.11. Although Scala 2.13.1 has been released, this change sticks with 2.13.0, as `scoverage` has not yet released an update compatible with 2.13.1: scoverage/sbt-scoverage#295 scoverage/scalac-scoverage-plugin#283 scoverage/sbt-scoverage#299 Migration issues addressed with the update to Scala 2.13: ### Underscore (`_`) is no longer a valid identifer for `val`s Using underscore like this is no longer allowed... ``` implicit val _ = ... ``` ...we have to give the `val` a valid name (even if it's just `v`): ``` implicit val v = ... ``` See also: * scala/bug#10384 * scala/bug#7691 * scala/scala#6218 ### `Map.mapValues()` now returns a `MapView` rather than a `Map` We usually want a `Map`, so we can just add in a `.toMap` to the end of the expression - it's harmless in Scala 2.12, and allows the code to compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala 2.13, but using the new recommended alternative doesn't work in Scala 2.12. https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes See also: * scala/bug#10919 * scala/scala#7014 ### `.to` can no longer infer what resulting collection type you want This is all part of `CanBuildFrom` going away in Scala 2.13: https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
This implements scala/bug#10919