-
Notifications
You must be signed in to change notification settings - Fork 9
Allow users to add custom units #58 #59
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
base: main
Are you sure you want to change the base?
Conversation
especially with countable amounts, like currency, cups etc. they are not interchangable and units can be used to distinguish them.
davidhassell
left a comment
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.
Hi @rju,
Thank you! I've make some suggestions/asked some questions, but it looks good and works as far as I can tell by playing on the command line (the proper units test will tell all :)).
Would you like add yourself as a contributor at docs/source/contributing.rst?
| # -------------------------------------------------------------------- | ||
| # Function to decode Udunits status codes to text | ||
| # -------------------------------------------------------------------- | ||
| def decode_status(status): |
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.
| def decode_status(status): | |
| def _decode_status(status): |
... and doc string, please!
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.
Can you provide an example of when this would ever get called? I see that we only ever call _ut_format if there already an integer self._ut_unit, so I'm wondering how the former could go wrong?
| return out.decode("utf-8") | ||
|
|
||
| @classmethod | ||
| def new_unit(cls, name): |
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.
Doc string please!
| @@ -0,0 +1,59 @@ | |||
| #!/usr/bin/env python3 | |||
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.
Would it be possible to recast this a new method in cfunits.cfunits.test.UnitsTest?
The tests need be framed with self.ssertTrue, self.assertFalse, self.assertEqual, etc.
| out = _string_buffer.value | ||
| else: | ||
| raise ValueError(f"Can't format unit {self!r}") | ||
| raise ValueError(f"Cannot format unit {self!r} cause: {decode_status(_ut_get_status())}") |
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.
| raise ValueError(f"Cannot format unit {self!r} cause: {decode_status(_ut_get_status())}") | |
| raise ValueError(f"Cannot format unit {self!r} cause: {_decode_status(_ut_get_status())}") |
This pull request adds the feature to add custom units programmatically. See also #58