Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

unit_system syntax overhaul, support metric prefixes #44

Merged
merged 83 commits into from
Dec 30, 2023
Merged

Conversation

Tehforsch
Copy link
Owner

Solves #18

term, this should be handled by the compiler.
… base dimensions as those without right hand side
…y. Dimension -> BaseDimension, Quantity -> Dimension
…ick when unknown identifiers are used in annotations, instead emit a proper error message
@Tehforsch Tehforsch added the enhancement New feature or request label Dec 29, 2023
//TODO(minor): Support using ° here.
#[symbol(deg)]
#[alias(degrees)]
unit degree: Angle = PI / 180 * radian;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
unit degree: Angle = PI / 180 * radian;
unit degree: Angle = 180 / PI * radian;

Copy link
Owner Author

Choose a reason for hiding this comment

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

As confusing as it is, degree = PI / 180 * radian is the correct definition. It feels really weird because it seems like it's multiplying radian with PI (which it already contains), but a degree is less than a radian, so PI / 180 is the correct factor.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gosh, never underestimate the power of covariance/contravariance to confuse. I don't know why it looked natural for the others, but jarring here. Sorry about the noise.

Copy link
Owner Author

Choose a reason for hiding this comment

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

No worries! When I saw your change request I was completely convinced that you were right and that the code was wrong, so I made exactly the same mistake. Thanks for looking through the definitions so thoroughly!

@Tehforsch Tehforsch merged commit 801f445 into main Dec 30, 2023
@Tehforsch Tehforsch linked an issue Dec 31, 2023 that may be closed by this pull request
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Properly set up SI module
2 participants