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

Avoid that formatted strings discard units of quantities #1377

Merged
merged 1 commit into from Dec 7, 2021

Conversation

mstimberg
Copy link
Member

The big string formatting unification in d3ae592 introduced a regression in a few places, because the default formatting for quantities used in f-strings or .format is to display it as a floating point number, discarding its units:

>>> start = 0*second
>>> duration = 1 * second
>>> print("Starting simulation at t=%s for a duration of %s" % (start, duration))
Starting simulation at t=0. s for a duration of 1. s
>>> print(f"Starting simulation at t={start} for a duration of {duration}")
Starting simulation at t=0.0 for a duration of 1.0

In most places, I worked around this by using something like "... t={start!s}" to manually force a conversion to a string, but I think it would be better if we changed quantities' default for "formatting without format spec" (which would also be more consistent with a simple print(start)). Luckily, we can do this easily using the __format__ function. With this PR, both print statements above give the same output.

@mstimberg
Copy link
Member Author

I'm going ahead with the merge as it should be low risk (and fixes the confusing output in a number of places, e.g. for run(...report='text'), or print(profiling_summary()). It would also be trivial to revert if we find any issue with it.

@mstimberg mstimberg merged commit 056e74c into master Dec 7, 2021
@mstimberg mstimberg deleted the format_units branch December 7, 2021 16:34
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

1 participant