Model.add_number and add_variable now allow unit objects as well as strings#172
Model.add_number and add_variable now allow unit objects as well as strings#172MichaelClerx merged 4 commits intodevelopfrom
Conversation
…trings as arguments.
MichaelClerx
requested review from
jonc125 and
skeating
and removed request for
skeating
Codecov Report
@@ Coverage Diff @@
## develop #172 +/- ##
===========================================
+ Coverage 95.89% 96.14% +0.25%
===========================================
Files 6 6
Lines 876 882 +6
Branches 182 184 +2
===========================================
+ Hits 840 848 +8
+ Misses 19 17 -2
Partials 17 17
Continue to review full report at Codecov.
|
jonc125
left a comment
jonc125
left a comment
There was a problem hiding this comment.
Looks good to me. Couple of minor suggestions; see what you think.
|
|
||
| :param number: A number (anything convertible to float). | ||
| :param units: A string unit representation. | ||
| :param units: A `pint` units representation |
There was a problem hiding this comment.
As this also accepts strings, should we keep that option in the docs too? Or hide it just for use by our tests?
There was a problem hiding this comment.
Oh, and should it be double backticks? My reST is rusty!
| ontology_terms.append(uri_parts[-1]) | ||
| return ontology_terms | ||
|
|
||
| def get_units(self, name): |
There was a problem hiding this comment.
Should we use this method above?
| :return: pint.Unit | ||
| throws pint.UndefinedUnitError if te unit is not present in registry | ||
| :return: `Unit` | ||
| throws pint.UndefinedUnitError if the unit is not present in registry |
There was a problem hiding this comment.
Could use the Sphinx :raises pint.UndefinedUnitError: if... syntax here, actually?
3 participants
See #132
Leaving in the option to use strings for now, as it's used in lots of tests etc. Might be nice for users too