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

Fix unit string representation #1291

Merged
merged 3 commits into from Apr 21, 2021
Merged

Fix unit string representation #1291

merged 3 commits into from Apr 21, 2021

Conversation

mstimberg
Copy link
Member

As @romainbrette noticed, the string representation of some units were not correct in the sense that evaluating them in Python did not result in the same unit – this is obviously bad, in particular when storing units/equations in text and importing them later (as Romain did). The problem concerned units raised to powers more than once, e.g.:

>>> (second ** 2) ** -1  # or 1/second**2 which will be interpreted in the same way
second ** 2 ** -1

Due to Python's operator precedence, this will be interpreted as second ** (2 ** -1), i.e. second ** 0.5 instead of second ** -2 as it should be. The simple fix in this PR is to consider units raised to powers as "compound units", which will lead to the use of parentheses:

>>> (second ** 2) ** -1
(second ** 2) ** -1

Note that this changes the output also in situations that do not need correction, e.g. amp / cm ** 2 will now be represented as amp / (cm ** 2). I'd rather keep the fix as simple as it is though, the new output might even be helpful to avoid confusion.

While fixing this I came across another minor issue which is now fixed as well: the function is_scalar_type was supposed to return True for a scalar that "can be interpreted as a dimensionless Quantity", but it actually only checked if it was a scalar, not whether it was dimensionless. Since this function was used to check whether an exponent of a unit was useable, it did give a somewhat confusing error message for nonsensical operations like second**second:

...
brian2.units.fundamentalunits.DimensionMismatchError: Cannot calculate 10.0 ** 0. s, the exponent has to be dimensionless (unit is s).

With the fix, the error message is more meaningful

brian2.units.fundamentalunits.DimensionMismatchError: Cannot calculate s ** s, the exponent has to be dimensionless (unit is s).

@mstimberg mstimberg merged commit 2cdf5d2 into master Apr 21, 2021
@mstimberg mstimberg deleted the unit_repr_fix branch April 21, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants