-
Notifications
You must be signed in to change notification settings - Fork 121
Migrations from Dotty #302
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
Conversation
- Rewrite `apply = parse _` to `apply(x: Any) = parse(x)`: scala/scala3#2994 - Remove self type: scala/scala3#2214
Thanks for your PR. It seems it covers various changes:
Is this correct? Perhaps we should split it in several PRs? |
Hi @cquiroz, This PR only covers two changes.
I can split the changes into two PR if you want.
Yes, it is possible to make |
Thanks for clarifying the scope of the plugin, it would be nice to add it to the Travis build but I guess it could be done in another PR |
@@ -13,7 +13,7 @@ package object strict { | |||
object implicits { | |||
implicit def mkSiUnitGroup[A <: Quantity[A]](implicit dimension: Dimension[A]): UnitGroup[A] = { | |||
new UnitGroup[A] { | |||
val units: Set[UnitOfMeasure[A]] = dimension.units.collect { case si: SiUnit => si } | |||
val units: Set[UnitOfMeasure[A]] = dimension.units.filter(_.isInstanceOf[SiUnit]) |
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.
Why is this change required? I'd rather not have isInstanceOf
in the code base
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.
This a bug in Dotty. See scala/scala3#3208. It is not clear if this will be fixed as one can always rewrite the code as:
dimension.units.collect { case si: UnitOfMeasure[A] with SiUnit => si }
However, I though using collect
here was overkill since what it really doe is filter
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'd rather have the collect
call than isInstanceof
@@ -47,7 +47,7 @@ final class Dimensionless private (val value: Double, val unit: DimensionlessUni | |||
*/ | |||
object Dimensionless extends Dimension[Dimensionless] { | |||
def apply[A](n: A, unit: DimensionlessUnit)(implicit num: Numeric[A]) = new Dimensionless(num.toDouble(n), unit) | |||
def apply = parse _ | |||
def apply(value: Any) = parse(value) |
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.
Is this change required by dotty?
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.
Yes. See this comment
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.
👍
Are there any other opinions about this PR? Otherwise, I could merge it |
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.
LGTM
This changes were needed to make squants compile with Dotty (as part of our community build). I though I might as well upstream them