CalebBell / thermo

Thermodynamics and Phase Equilibrium component of Chemical Engineering Design Library (ChEDL)
MIT License
594 stars 114 forks source link

Optionally raise exception for T and TPDependentProperty objects, and many fixes! #82

Closed yoelcortes closed 3 years ago

yoelcortes commented 3 years ago

Hi Caleb,

This pull request implements a "RAISE_PROPERTY_CALCULATION_ERROR" class attribute that serves as a flag to raise exceptions when no value could be calculated in T and TPDependentProperty, and MixtureProperty objects. You can run the new tests I added to view what the errors look like.

I'd like to add this new feature for developer debugging, better error reporting, and to help users navigate through the many reasons they can get a value of None. Having these kinds of errors helped a ton in BioSTEAM, letting users know quickly when no method is available, if the temperature was outside the domain, or if the property value is nonsensical.

I chose a simple implementation, but there are a couple of other ways to implement the optional exceptions, including:

Let me know if you believe a different implementation may be better.

In addition to this feature, this pull request also made the following fixes:

Again, no rush on this pull request.

Looking forward to hearing your thoughts, Thanks!

CalebBell commented 3 years ago

Hi Yoel, This hasn't been the best merge process ever. In general I think the feature is a fair idea. I am hesitant to document this and show it to users until you are happy with its behavior, so let's merge it as is and let this feature "grow up" a little under your watch! We can offer it to users at a later point.

I also did some surprisingly painful work on this branch to get more tests of the documentation working. The code coverage should be more accurate now.

Sincerely, Caleb

yoelcortes commented 3 years ago

@CalebBell, stellar work!

With this merge I believe I'm all set to start coding in thermo as a hard dependency in BioSTEAM. Rest assured, this feature will be used a lot and should mature soon enough to document it.

Thank you again!