You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When formatting a unit that has a complex value, parentheses are only added around the value when the value has both a non-zero real and imaginary part.
You need to add a test for format on (1 + 1e-15i) kW or something like that. We want that to format to 1 kW because the complex part is negligible. But I believe it is going to fail, because you're checking if nearlyEqual(1e-15, 0), which is false, because the default abstol is 0, and reltol can't ever think something is sufficiently close to 0 in particular.
The upshot is that (currently) testing if a complex number is actually real or actually pure imaginary is tricky. My recommended method is to test if z is real via math.scalarEqual(z.re, z.re + z.im), using the mathjs function scalarEqual which in turn respects the configured tolerances. And then !math.scalarEqual(z.re, z.re+z.im) && math.scalarEqual(z.im, z.re + z.im) tests if it is pure imaginary. You need both tests because otherwise complex zero would be considered pure imaginary when in fact it is real. You can of course use equal instead of equalScalar because Unit.js already depends on equal, but the equalScalar is a bit simpler and may be slightly faster.
So perhaps actually we should add mathjs functions isReal() and isPureImaginary() so that we have an established, uniform way of testing complex numbers for these properties. You could try to push these properties back to Complex.js, but then of course that can't employ the mathjs tolerance system.
Sorry this opened a little bit of a can of worms. But I don't think this PR is ready to merge as it stands, unfortunately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3614
When formatting a unit that has a complex value, parentheses are only added around the value when the value has both a non-zero real and imaginary part.