CalebBell / chemicals

chemicals: Chemical database of Chemical Engineering Design Library (ChEDL)
MIT License
186 stars 36 forks source link

Maintaining compatibility with magic pint parser #5

Closed CalebBell closed 4 years ago

CalebBell commented 4 years ago

Hi Yoel,

A long time ago I put a lot of effort into the docstring format I was using, and made a reader and shim to make the functions I was writing in all my projects work with pint. I just added it to chemicals also, but you can see more details here: https://fluids.readthedocs.io/fluids.units.html

You can try it out on master as follows; the return type will be a pint value.

from chemicals.units import Tc Tc('64-17-5')

I use this framework pretty often because I like doing calculations with pint.

I feel keeping pint as an optional dependency is a good one, and keeping it out of the library internals avoids slowing things down and allows it to be switched out for another library in the future.

However, the return signature of a function is expected to be what it says in the documentation. This is just one more complication to changing things.

yoelcortes commented 4 years ago

Hmm, so you're saying that the documentation needs to match for all functions so that the units wrap can work, is this correct? If so, I dont really see a problem with this.

In any case, I think we can handle compatibility issues for this. The units interface seems pretty useful for sanity checks, especially when its not just for research.

yoelcortes commented 4 years ago

I think I did something similar, but with only the signature of the functions to make a consistent documentation of functors. If you look at thermosteam.base.documenter and thermosteam.base.units_of_measure, you might get an idea of what I did. This was just used to generate the docstrings, but I copy-pasted the docstrings to each functor.

In the same spirit, one option could be to have a consistent set of units of measure for each variable and take advantage of this in the signature of your function. Then all we need to do is wrap the methods we'd like to see with this "units" behavior:

# In the interface.py module
@units(returns='N/m')
def Brock_Bird(T, Tb, Tc, Pc):
    ...
    return sigma
>>> # a not so good example
>>> from chemicals.units import *
>>> from pint import _DEFAULT_REGISTRY as u
>>> Brock_Bird(100*u.degC, 100*u.degC, 373.9*u.degC, 2*u.atm)
Quantity(-0.00022503186589661344, 'N/m')

If for some reason, the units are not consistent in some functions, we could also note the units in the units decorator:

# In the interface.py module
@units(returns='N/m', Pc='atm')
def Brock_Bird(T, Tb, Tc, Pc):
    ...
    return sigma

The data passed to the decorator can later be used by the ureg.wraps method when loading the chemicals.units module. This would be pretty easy for me to implement, probably just a couple of hours at most. Let me know if you are interested.

CalebBell commented 4 years ago

Hi Yoel, I am quite happy with the string-based parsing I have in the docstrings at present. I would want the documentation to keep having the units also, which would be duplicating the information twice. There is also no overhead of the decorator this way.

I am just mentioning this to explain some of the docstring format, I consider it a pretty important feature. I believe I do have a consistent set of units of measure - base SI (kg, s, m, mol, etc.) for everything; with the exception of dipole moment, which has weird units. Please let me know if you see anything inconsistent.

yoelcortes commented 4 years ago

Ahh I see, I thought you may want to relieve the documentation format for some reason. I don't see a problem with the way it is either... I like units in brackets and adding the Returns section is cool too