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

Gather factors and add labels for scaled units #332

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

chiphogg
Copy link
Contributor

First, when multiplying a ScaledUnit by another magnitude, we now fold
it into the existing magnitude of the scaled unit. Previously, we'd end
up with ScaledUnit<ScaledUnit<U, M1>, M2>, and so on. We also now
omit any "trivial" scaling factors, whether because we're scaling by
mag<1>(), or (more commonly) whether we've applied a bunch of
different scale factors and they all cancel out.

(We did need to tweak a few cases that were relying on U{} * mag<1>()
being meaningfully different from U{}.)

Next, we now auto-generate labels for ScaledUnit specializations. For
ScaledUnit<U, M>, if U has label "U", and M has label "M", we
generate a label "[M U]" --- or, if "M" contains an exposed slash
"/", we'll generate the label "[(M) U]" for lack of ambiguity. This
resolves the vast majority of [UNLABELED_UNIT] labels. The remaining
work on #85 is simply to generate labels for a wider variety of
magnitude label categories.

Finally: we formerly had no way to decide ordering between units that
are both specializations of ScaledUnit, which do have identical
dimension and magnitude, and yet are not the same unit. (For
example, something like "[(1 / 4) ft]" and "[3 in]".) This may have
been somewhat obscure in the past, but with the upcoming work on #105,
it's about to become very common. We added a test case that exposes
this, and then updated our ordering code to handle this case.

Helps #85. Unblocks #105.

First, when multiplying a `ScaledUnit` by another magnitude, we now fold
it into the existing magnitude of the scaled unit.  Previously, we'd end
up with `ScaledUnit<ScaledUnit<U, M1>, M2>`, and so on.  We also now
omit any "trivial" scaling factors, whether because we're scaling by
`mag<1>()`, or (more commonly) whether we've applied a bunch of
different scale factors and they all cancel out.

(We did need to tweak a few cases that were relying on `U{} * mag<1>()`
being meaningfully different from `U{}`.)

Next, we now auto-generate labels for `ScaledUnit` specializations.  For
`ScaledUnit<U, M>`, if `U` has label `"U"`, and `M` has label `"M"`, we
generate a label `"[M U]"` --- or, if `"M"` contains an exposed slash
`"/"`, we'll generate the label `"[(M) U]"` for lack of ambiguity.  This
resolves the vast majority of `[UNLABELED_UNIT]` labels.  The remaining
work on #85 is simply to generate labels for a wider variety of
magnitude label categories.

Finally: we formerly had no way to decide ordering between units that
are _both_ specializations of `ScaledUnit`, which _do_ have identical
dimension _and_ magnitude, and yet are _not_ the same unit.  (For
example, something like `"[(1 / 4) ft]"` and `"[3 in]"`.)  This may have
been somewhat obscure in the past, but with the upcoming work on #105,
it's about to become very common.  We added a test case that exposes
this, and then updated our ordering code to handle this case.

Helps #85.  Unblocks #105.
@chiphogg chiphogg added the release notes: ✨ lib (enhancement) PR enhancing the library code label Nov 29, 2024
@chiphogg chiphogg requested a review from geoffviola November 29, 2024 13:10
Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cleaner. Thanks for the changes.

@chiphogg chiphogg merged commit e88baac into main Dec 2, 2024
13 checks passed
@chiphogg chiphogg deleted the chiphogg/scaled-unit#85 branch December 2, 2024 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: ✨ lib (enhancement) PR enhancing the library code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants