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

Real Quantities #694

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Real Quantities #694

wants to merge 1 commit into from

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented Nov 11, 2023

After generally-supportive discussion in #680, I wanted to see how feasible it is to make Quantity <: Real without breaking changes. It required few specialized changes, and basically all tests aside from logarithmic and affine pass (I just didn't touch them at all).

Overall I'm pretty positive that this change can be done without modifying tests at all (aside from those that check for specific subtyping of course), so no breaking changes.
Almost no special complex-of-quantity handling was required here, and working with complex units remain convenient with the same syntax.

Let me know if something along these lines can be accepted to Unitful, after updating the remaining parts so that all tests pass (currently some are commented out).

@giordano giordano added the v2.0 label Nov 13, 2023
@sostock
Copy link
Collaborator

sostock commented Dec 2, 2023

Overall I'm pretty positive that this change can be done without modifying tests at all (aside from those that check for specific subtyping of course), so no breaking changes.

Even if all of this package’s tests pass, that would not automatically mean that this isn’t a breaking change. We also need to make sure to not break anybody else’s code.

Here is a piece of code that works right now but doesn’t after this PR:

using Quaternions, Unitful

Quaternion(1,2,3,4)u"m"

One way to fix this particular case would be to make the unit m also a real number, i.e., m would be the same as 1m. Then we could at least still multiply any number type by units as long as that number type defines multiplication with real numbers. If a number type doesn’t define multiplication by real numbers, it would still be broken.

@aplavin
Copy link
Contributor Author

aplavin commented Dec 2, 2023

make the unit m also a real number, i.e., m would be the same as 1m

Nice approach!

An advantage of real quantities & units would be that then you'd be able to pass unitful numbers to f(::Quaternion) (and f(::Complex) and others). Now there's no way to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants