Skip to content

Conversation

lipchev
Copy link
Collaborator

@lipchev lipchev commented Jan 19, 2025

This is a port of #1393, targeting the release-v6 consisting of the changes that didn't require any modifications to the tests.

I'll create another PR with the rest of the (breaking) changes in a bit.

Copy link

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

I'll go ahead and merge this, just one minor comment/suggestion.

Tons of great work here 👏
I only skimmed the new constants though and spot tested a few places, but I have not verified every single change since it seems the tests still pass and that is a good indicator things are in order.

"FromUnitToBaseFunc": "{x} * Math.Pow(2.54e-2, 6)",
"FromBaseToUnitFunc": "{x} / Math.Pow(2.54e-2, 6)",
"FromUnitToBaseFunc": "{x} * 0.000000000268535866540096",
"FromBaseToUnitFunc": "{x} / 0.000000000268535866540096",
Copy link
Owner

Choose a reason for hiding this comment

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

In a way, I kind of prefer the Math.Pow formula, since it is easier to recognize the constant 2.54 and even repeating it 6 times 2.54e-2 * 2.54e-2 * ... could be one option if Math.Pow throws a curveball.
I have no strong opinion here, just an observation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can work with Math.Pow as well, but if we (or whoever) wanted to created a generator for another language, the uses of Math means more hoops to deal with..

@angularsen angularsen force-pushed the unit-conversion-expressions-v6 branch from 0d23892 to e4f6e62 Compare March 23, 2025 17:14
@angularsen angularsen merged commit 5ea0939 into angularsen:release/v6 Mar 23, 2025
1 check was pending
@angularsen angularsen changed the title Decomposed the rounded expressions in the unit definitions into operations with exact coefficients (non-breaking) Decompose unit conversion functions to exact (non-breaking) Mar 23, 2025
@angularsen angularsen changed the title Decompose unit conversion functions to exact (non-breaking) Decompose unit conversion functions to exact coefficients (non-breaking) Mar 23, 2025
@lipchev lipchev deleted the unit-conversion-expressions-v6 branch April 11, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants