Skip to content
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

Make viewness of mapValues / filterKeys more obvious #10919

Closed
lrytz opened this issue Jun 4, 2018 · 12 comments
Closed

Make viewness of mapValues / filterKeys more obvious #10919

lrytz opened this issue Jun 4, 2018 · 12 comments

Comments

@lrytz
Copy link
Member

lrytz commented Jun 4, 2018

@Ichoran suggests to rename them to mappedValueView / filteredKeyView (#4776 (comment)).

For naming, I think mapValuesView / filterKeysView would be more consistent, as the strict methods are called map / filter (not mapped / filtered).

"Alternatively, return a value class with a single method, view, that gives out the view"

@smarter
Copy link
Member

smarter commented Jun 6, 2018

Having a strict version of these methods would be great too.

@lrytz
Copy link
Member Author

lrytz commented Jun 6, 2018

In principle the operations should be strict on strict collections, and lazy on views, like other operations. We probably cannot just change the semantics though..

@szeiger
Copy link

szeiger commented Jun 6, 2018

Let's deprecate the operations on MapOps now and undeprecate them on MapView so that you get the desired semantics with .view.mapValues and .view.filterKeys. Maybe we can make the versions on Map strict in 2.14 after a deprecation cycle without removing them first? Otherwise this will have to wait until Scala 3.1.

@lrytz
Copy link
Member Author

lrytz commented Jun 6, 2018

This sounds good, but it's annoying if you want a strict mapValues (without deprecation warning): you have to write .view.mapValues(f).toMap. We could add mapValuesStrict? But it's also not great to add a method only to deprecate it in the next release...

@szeiger
Copy link

szeiger commented Jun 7, 2018

Well, it's already better than 2.12 where there's no obvious way at all to make the result strict.

@joshlemer
Copy link

@szeiger do we still want to deprecate MapOps#mapValues?

@szeiger
Copy link

szeiger commented Jul 19, 2018

After rereading this thread it still looks like a sensible option to me. I'm undecided whether the replacement should be a dedicated mapValuesView method or .view.mapValues.

@julienrf
Copy link

Introducing mapValuesStrict or mapValuesView would look quite bad, imho.

@SethTisue
Copy link
Member

@joshlemer once you've accepted the invitation to https://github.com/orgs/scala/teams/contributors , comment here and I'll assign this ticket to you.

@joshlemer
Copy link

@SethTisue I accepted it

@SethTisue
Copy link
Member

👍 thanks for all the good scala/scala PRs lately, much appreciated!

rtyley added a commit to guardian/twitter4s that referenced this issue Oct 20, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 20, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 20, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 20, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 21, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 21, 2019
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
rtyley added a commit to guardian/twitter4s that referenced this issue Oct 21, 2019
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
@SethTisue
Copy link
Member

SethTisue commented Apr 19, 2020

it's annoying if you want a strict mapValues (without deprecation warning): you have to write .view.mapValues(f).toMap

And also note that by going through .view like that you lose the original collection type. toMap will only be the correct rewrite if the input type was immutable.Map

fwiw, transform exists as a strict, collection-type-preserving alternative:

scala 2.13.1> collection.immutable.SortedMap(1 -> "foo")
res7: scala.collection.immutable.SortedMap[Int,String] = TreeMap(1 -> foo)

scala 2.13.1> res4.transform((_, v) => v.length)
res8: scala.collection.immutable.SortedMap[Int,Int] = TreeMap(1 -> 3)

thx @OlegYch for the reminder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants