You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sis.apache.org by GitBox <gi...@apache.org> on 2022/03/28 15:58:28 UTC
[GitHub] [sis] alexismanin opened a new pull request #26: Feat/acceleration units
alexismanin opened a new pull request #26:
URL: https://github.com/apache/sis/pull/26
Add base acceleration unit (m/s²) and a derivative: [GAL](https://fr.wikipedia.org/wiki/Gal_(unité))
Bonus: fix a typo on french name for degree unit
@desruisseaux : I'd like a review from you please :
1. Are y additions well-defined ?
2. On the fix about degree unit, there's a point I do not understand : In some tests (that do **not** fail), we expect something that is a mix of french and english : UnitTest#testValueOf(), lines 273, 275, 278. I do not know if I should make a change here or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sis] desruisseaux commented on a change in pull request #26: Feat/acceleration units
Posted by GitBox <gi...@apache.org>.
desruisseaux commented on a change in pull request #26:
URL: https://github.com/apache/sis/pull/26#discussion_r838352900
##########
File path: core/sis-utility/src/main/java/org/apache/sis/measure/Units.java
##########
@@ -1290,6 +1303,13 @@
DECIBEL = add(bel, Prefixes.converter('d'), "dB", ACCEPTED, (short) 0);
UNITY = UnitRegistry.init(one); // Must be last in order to take precedence over all other units associated to UnitDimension.NONE.
+ /*
+ * Acceleration units
+ */
+ METRES_PER_SECOND_SQUARED = mps2;
+ mps2.related(1);
+ GAL = add(mps2, centi, "gal", (byte) (OTHER | PREFIXABLE | ACCEPTED), (short) 0);
Review comment:
We have a tricky situation here. According Wikipedia (I verified also in BIPM), the symbol is "Gal" (upper-case letter) but the name is "gal" (lower-case letter). The argument in `add` method call is the symbol, so it should have an upper-case letter. But we need to add an entry in the localized resource files for formatting the name with lower-case letter when writing as text.
Another topic is that this units is part of the centimeter–gram–second (CGS) system. This is the first occurrence of this system in our `Units` class, but CGS may be important enough to deserve a new constant in `UnitRegistry` that we should use instead of `OTHER`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sis] desruisseaux commented on a change in pull request #26: Feat/acceleration units
Posted by GitBox <gi...@apache.org>.
desruisseaux commented on a change in pull request #26:
URL: https://github.com/apache/sis/pull/26#discussion_r838339277
##########
File path: core/sis-utility/src/main/java/org/apache/sis/measure/Scalar.java
##########
@@ -539,4 +540,17 @@ public final String toString() {
return new Temperature(value, unit);
}
}
+
+ static final class Acceleration extends Scalar<javax.measure.quantity.Acceleration>
Review comment:
This definition can be moved just below `Speed` for keeping related quantities together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sis] desruisseaux commented on a change in pull request #26: Feat/acceleration units
Posted by GitBox <gi...@apache.org>.
desruisseaux commented on a change in pull request #26:
URL: https://github.com/apache/sis/pull/26#discussion_r838340810
##########
File path: core/sis-utility/src/main/java/org/apache/sis/measure/Units.java
##########
@@ -1111,6 +1111,16 @@
*/
public static final Unit<Dimensionless> PIXEL;
+ /**
Review comment:
Those fields can be moved below `KILOMETRES_PER_HOUR` for keeping related units together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sis] asfgit merged pull request #26: Feat/acceleration units
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #26:
URL: https://github.com/apache/sis/pull/26
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org