search

unitsofmeasurement / unit-api

Units of Measurement API
http://unitsofmeasurement.github.io/unit-api/
Other
183 stars 42 forks source link

A series of with() methods instead of prefix(), transform() and mix()? #189

Closed keilw closed 5 years ago

keilw commented 5 years ago

Based on suggestions by @dautelle in https://github.com/unitsofmeasurement/unit-api/issues/185 or https://github.com/unitsofmeasurement/unit-api/issues/187 I would like to propose a more consequent approach that's partly inspired by java.time.temporal.Temporal.

Instead of a lonely with() method that does not really mean anything, and would seem rather inconsistent, we could define

Unit<Q> with(UnitConverter converter);
Unit<Q> with(Prefix prefix);
Unit<Q> with(Unit<Q> that);

And deprecate 

Unit<Q> transform(UnitConverter operation);

in favor of with(UnitConverter).

The others are new and have not been around in JSR 363, so they are still subject to change. Most JSR 310 key elements including the package name change happened at a very late stage before Java 8 and its 2 EDRs or Public Review looked significantly different to what we know now.

WDYT?

keilw commented 5 years ago

Everyone who voted on #184 (@otaviojava, @thodorisbais, @mohamed-taman, @filipvanlaenen, @Daniel-Dos) sorry to bother you again, just wanted to bring it up while there is still time to do those kinds of changes, if this ticket was ignored then I guess we'll stick to the 3 methods above.

teobais commented 5 years ago

Conceptually, it's not that wrong. On the other hand, it doesn't seem like rational enough to convince my eyes. I mean, the purpose of the currently used methods (prefix(), transform() and mix()) is quite clear, whereas overriding all of them with with() will create an additional action/thought to the user: that one of reading the method's signature, especially when used together. Example:

IMO 

Unit.with(prefix).with(unitConverter)
requires more effort from the user than

Unit.prefix(prefix).transform(unitConverter)
. Undoubtedly, this is a small detail, which however, doesn't let me finalize my vote yet. @dautelle do you have some more arguments here so that I could be convinced? :-)
dautelle commented 5 years ago

Hi Thodoris,

I think that 'MixedUnit' are useful since you find them everywhere (durations, angles, etc.). My only issue is with the method name 'mix' which also means to blend (which is the opposite of what we mean). The best name would be'compound' but it is already used in a different context. The methods names 'plus', 'and', 'join'.. are ok too

On Sat, Mar 30, 2019 at 9:56 AM Thodoris Bais notifications@github.com wrote:

Conceptually, it's not that wrong. On the other hand, it doesn't seem like rational enough to convince my eyes. I mean, the purpose of the currently used methods (prefix(), transform() and mix()) is quite clear, whereas overriding all of them with with() will create an additional action/thought to the user: that one of reading the method's signature, especially when used together. Example:

IMO

Unit.with(prefix).with(unitConverter)

requires more effort from the user than

Unit.prefix(prefix).transform(unitConverter).

Undoubtedly, this is a small detail, which however, doesn't let me finalize my vote yet. @dautelle do you have some more arguments here so that I could be convinced? :-)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/unitsofmeasurement/unit-api/issues/189#issuecomment-478222317, or mute the thread https://github.com/notifications/unsubscribe-auth/AF59KlVYhoMfCjmLchcPH_dbZeM0BcHeks5vbybbgaJpZM4cTanz .

desruisseaux commented 5 years ago

In the particular case of angles, degrees mixed with minutes and seconds are named "Sexagesimal degrees" in the EPSG Geodetic registry (code 9110) and are considered as pseudo-units, not real units. EPSG considers "mixed units" as changes of base ("sexagesimal" stands for "base 60"), not different units.

keilw commented 5 years ago

the most pseudo-unit is the "annotated" unit, and yet we have them in several implementations and at least the question was asked in https://github.com/unitsofmeasurement/unit-api/issues/54 whether to expose it in an implementation neutral way. That ticket has just 2 votes in favor, "mix" has 4 against 1 so more people believe it makes sense, a few more even found mix() the best alternative in #184.

It was a thought to make something out of the with() suggestion, but it seems not many find it appealing. If we agree a "MixedUnit" or "mixed..." was a good thing to keep, than what is the logic of calling a method that creates those kinds of methods something completely different? Or would you call "MixedUnit" a "JointUnit" in case of join()? @thodorisbais We might find a few extra users of the JSR in Amsterdam if we went that way? ;-) "join" usually gets used in queries, but it would be less confusing in this context, either way if the corresponding JavaDoc talks about "mixed units" then it would be fairly unclear why it was named that way. Remember BigDecimal.ulp()? What does it do without looking up the JavaDoc or source code? ;-) plus() was used instead of add() or shift() earlier, that I'm afraid was used in a completely different context. Instead of more votes if there really is a "one name to mix them all" and one of you thinks it also could make sense with JavaDoc, please propose a PR here for Unit. And ask all 3 Spec Leads to review.

185 has enough votes (even the 5 JCP rules would require) and is far from a tie, all the others are not conclusive enough right now.

Another metaphor from SQL and queries beside join would be group. A "GroupedUnit" could also work even

184 was also clear, which is why it may seem to disrespect a majority vote. We prefer to not have a Spec Lead dictate things, the mentioned 310 or some JDK JSRs probably did that slightly different, but we at least have 3 to decide where necessary.

desruisseaux commented 5 years ago

@keilw : annotated units said something about the measurement method (percentage of mass versus percentage of volume), or change the conversion factor (standard mile versus US survey mile).

I ask technical questions. Please give technical answers. No authority argument, no "Indriya does that", no "this issue has more vote".

keilw commented 5 years ago

Yes of course it does, it combines the factors of the individual units in the "mix / group" or whatever we end up calling it. @desruisseaux I think you may mistake AlternateUnit and AnnotatedUnit. The AnnotatedUnit simply puts an extra string on an existing unit, no change of behavior, thus unlike a mixed/grouped/joint unit where the combination changes all relevant operations and conversions this unit implementation is pretty much a formatting aspect only. In a democratic process of such working groups neither @dautelle, @otaviojava, @desruisseaux or anybody else single-handedly dictates what to put into the API or not. So yes the issue that has more + votes in the end gets resolved, others are not important enough and may be revisited in a future version.

keilw commented 5 years ago

Closing this as it seems to not find a consensus.

desruisseaux commented 5 years ago

Thanks for the technical part of the answer. You said:

(mixed unit) combines the factors of the individual units in the "mix / group"

Can you elaborate on that? Given conversion factors of 0.3048 metres for FOOT and 0.0254 metres for INCH, how those conversion factors are mixed in FOOT.mix(INCH)? For example what is the difference between the followings?

the combination changes all relevant operations and conversions

Please elaborate. What is changed between those two operations?

Aside (following is not part of technical discussion, let not be distracted too much by that):

keilw commented 5 years ago

We also found that 1°C could be the same as 274.15° but that's not how votes work (except in Crimea or somewhere in the Deep US South maybe ;-)

I don't have time for every detail because I have to travel to a new client tomorrow. Please refer to MixedUnitTest for a detailed analysis of how the result of a mix() operation behaves.

There are certain operations that are harder to apply to every single underlying unit, e.g. the mentioned pow() or a Prefix like MEGA or CENTI.

So as for #185 and the support for mixed units in general, @unitsofmeasurement/experts, @unitsofmeasurement/contributors please advise, would you like to scrap mixed units altogether and leave this to each individual application to juggle with arrays and collections of Unit and Quantity values themselves? It's your call, please state your preference in #185 not here.

desruisseaux commented 5 years ago

We will never agree on "mathematic can not be voted" topic. Lets keep that aside.

My objections are not against "mixed unit". It is about handling them as a fundamental unit property instead than a formatting option.

At the very least, current Unit.mix(Unit) proposal is ill-defined. We still have no method contract, no answer other than "see Indriya" to most technical questions about method behavior.

We are turning in circles. I don't know what to do for bringing this discussion on technical grounds. Should we create a wikipage where I list all technical questions, where peoples can insert answers?

keilw commented 5 years ago

I'm afraid that's a bit late, but offering the formatting aspect only (it still must know how to format, this works 100% fine for both formatting and parsing by using the two types as Meta Information) could be a compromise, then I guess that would still work because throwing everything in the RI away once again or shelve it in a branch. In the end it's never just formatting because formatting takes an actual Quantity whatever concrete nature and also must allow t parse it. This is completely done, the only thing that's missing would be disecting 1.4 feet into feet and inch.

keilw commented 5 years ago

Anyway let's keep the discussions in the open threads please, #185 and to a lesser extent #184.

dautelle commented 5 years ago

Agreed. It might be an extension out of the scope of the Unit API, since Mixed/Multi Units are not real units (they deal with tuples not numbers).

On Sat, Mar 30, 2019 at 4:56 PM Martin Desruisseaux < notifications@github.com> wrote:

We will never agree on "mathematic can not be voted" topic. Lets keep that aside.

My objections are not against "mixed unit". It is about handling them as a fundamental unit property instead than a formatting option.

At the very least, current Unit.mix(Unit) proposal is ill-defined. We still have no method contract, no answer other than "see Indriya" to most technical questions about method behavior.

We are turning in circles. I don't know what to do for bringing this discussion on technical grounds. Should we create a wikipage where I list all technical questions, where peoples can insert answers?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/unitsofmeasurement/unit-api/issues/189#issuecomment-478258808, or mute the thread https://github.com/notifications/unsubscribe-auth/AF59KnlyKYj8D6R7pL2a9TvA8c09ljECks5vb4kigaJpZM4cTanz .

keilw commented 5 years ago

Well this CompoundQuantity behaved completely different, yet for a very similar purpose. It can be created from an array of matching quantities, thus the need of a CompoundUnit or MixedUnit was not given in that branch from what I remember.