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

Decompose unit conversion functions to exact coefficients (breaking) #1499

Open
wants to merge 5 commits into
base: release/v6
Choose a base branch
from

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 broke one test or another.

Most of these are related to conversions involving the US/British units but there is also some confusion regarding the Calorie - the value of 4.1868 was used (instead of 4.184) in a few places, which is a note-worthy difference.

Here are all the tests that broke:

@lipchev
Copy link
Collaborator Author

lipchev commented Jan 19, 2025

I still want to replace the PressureUnit.FeetOfElevation and PressureUnit.MeterOfElevation with properties that convert to/from Length (see #1393 (comment)) and remove the Frequency.BUnit but I'll open separate PRs for these (didn't want to have the Units.g.json changed here).

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.

@angularsen angularsen changed the title Decomposed the rounded expressions in the unit definitions into operations with exact coefficients (breaking) Decompose unit conversion functions to exact coefficients (breaking) Mar 23, 2025
@angularsen angularsen force-pushed the fix-unit-conversions branch from 5da8e22 to 80d3692 Compare March 23, 2025 17:16
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.

Not through everything yet but posting the review so far.
I got through all the JSON files.

…umetricHeatCapacity and Power

- HydraulicHorsepower: conversion coefficient changed from {x} * 745.69988145 to 745.69987158227022
@lipchev lipchev requested a review from angularsen March 24, 2025 07:42
@lipchev
Copy link
Collaborator Author

lipchev commented Apr 2, 2025

@angularsen I've merged the modifications from release/v6, this should be ready to review- please tell me if there is something else you want me to do.

I'd really like to close this as soon as possible- it's blocking my 🐲 PR..

@angularsen
Copy link
Owner

I'll make an honest attempt the next few days, I know I'm running late on a bunch of PRs lately and it's on my conscience, I just struggle to find the time and energy.

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