CovertLab / wcEcoli

Whole Cell Model of E. coli
Other
18 stars 4 forks source link

reconsider Unum units package #426

Open 1fish2 opened 5 years ago

1fish2 commented 5 years ago

PR #423 adds a temporary monkey-patch to the unum units package to fix a bug -- the Unum class doesn't implement true-division when the Python 2 caller has from __future__ import division.

See the Unum bug report.

That bug impedes Python 3 compatibility which is a necessary step for switching to Python 3.

The comment at the top of its main source file __init__.py reads:

# TODO: consider alternatives to unum and see how they compare.
1fish2 commented 5 years ago

Merging in what Eran wrote in #439:

This issue follows up on some recent conversations with @prismofeverything and @heejochoi about our units package, unum. #426 mentions that unum is no longer maintained, and is incompatible with Python 3.

There are additional behaviors that make it less-than-desirable. These include:

  • units are given to each value in an array, rather than the array itself. This becomes costly as the arrays are passed around.
  • units aren't saved in our listeners
  • there are various non-commutative operations that lead to strange units. We need better handling of conversions.

That being said, I appreciate having units available as much as possible, so I do not want to simply remove unum. Maybe we can modify unum to fit our needs, or write our own units package.

Additional details:

1fish2 commented 3 years ago

The original unum source code project is no longer online at https://pypi.org/project/Unum/ . The pip is still in PyPI.

There is a fork at https://github.com/trzemecki/Unum

The project should probably switch to pint==0.16.1 as used in Vivarium. requirements.txt already has Pint==0.14.