Skip to content

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