-
Notifications
You must be signed in to change notification settings - Fork 10
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
set pint default unit format to short notation #519
set pint default unit format to short notation #519
Conversation
@@ -454,7 +455,7 @@ def test_get_signal_data(self): | |||
mc.add_transformation(self._default_transformation(), data=data) | |||
mc.create_transformation("transformation_2", None, output_signal_unit="A") | |||
|
|||
assert mc.get_signal_data("transformation").identical(data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not really sure why this worked in the first place since get_signal_data
is supposed to return a TimeSeries
(which doesn't have .identical
) 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that what the type hint says or the actual code? Maybe the type hints or the code need an adjustment too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the type hint says TimeSeries
but I didn't check any further (the code looks more like xarray
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... strange. Will have to check this on my own. Usually, this should be caught by mypy
Codecov Report
@@ Coverage Diff @@
## master #519 +/- ##
==========================================
- Coverage 95.15% 95.13% -0.02%
==========================================
Files 93 93
Lines 5638 5637 -1
==========================================
- Hits 5365 5363 -2
- Misses 273 274 +1
Continue to review full report at Codecov.
|
# Conflicts: # weldx/constants.py
# Conflicts: # weldx/tags/unit/pint_quantity.py
updated to use short notation for serialization, see above |
Changes
use
pint
short notation for units as defaultI encountered some problems with serialization using short notation (not sure why), so explicitly keeping the long notation thereSince tests are running fine now I have explicitly set the format for
pint.Unit
serialization to short notation, I think this is the most concise (and readable) notation in the file.The user can still change the default formatting of the
WELDX_UNIT_REGISTRY
but it will not affect the serialization outputChecks