CalebBell / thermo

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

Add conda as installation option #21

Closed volpatto closed 5 years ago

volpatto commented 5 years ago

Hi, @CalebBell.

First of all, congratulations for the awesome work done in this lib (and in the others as well)! We talked in the past, but I always want to emphasize your work.

This PR adds conda as an installation option. I already did the work to add thermo to conda-forge recipes (as well as for fluids).

I don't know if there is other parts containing installation instruction, so please point it out if I missed something.

I hope you would appreciate.

CalebBell commented 5 years ago

Hi Diego, Good to hear from you again!

Conda is very nice... I get quite a few of the packages from conda. I have tested the packages and it looks like they're working! Thank you, I have merged it.

Sincerely, Caleb

volpatto commented 5 years ago

@CalebBell, thanks for replying!

Do you mind with I try to fix some tests? Locally, installing with pip, some tests are breaking, mostly due to tight allclose_assert tolerances. Do you have such behavior in your machine locally? There is any problem if I try to figure it out how to fix, if you are facing the same problems?

Ah, one last thing: If you would like to be included in the thermo's conda feedstock maintainers (actually it has only me), I can include you. If you don't want it (yes, one more thing to maintain, I know...), no problem, this is related only to installation, not the package itself, I can handle it, no problem!

Best regards, Diego.

CalebBell commented 5 years ago

Hi Diego,

Yes I would appreciate being added to the conda packages; thank you. I may try to make one for ht as well eventually, the third of the trio of my main repositories. It looks like upgrading them is straight forward enough.

I'm not sure what tests are failing for you. I pushed a few commits to get the CI environments working again to know for sure, and they are all passing their now after a few changes. They are very fragile unfortunately, but they are important because they always get the latest versions of numpy, scipy, etc - and that tells me if I'm out of date.

I have had precision trouble before but generally I prefer to keep a very tight tolerance on the tests. One molar volume method may be very similar to another, differing by 0.1% - it is important that the tests can tell the difference between the two, otherwise one may fail, the next be called, and the test will still pass.

You may be running tests without some of the dependencies, or with older versions of them? Sympy, CoolProp, and pint are required by some tests. I try to keep the test results so that they always pass on the latest versions of the dependencies.

Sincerely, Caleb

volpatto commented 5 years ago

Caleb,

I will try (I need to check the correct way to do it) do add you as a maintainer in the thermo (and fluids) conda-forge's feedstock today (monday). If you wish, I can help you with putting ht into conda-forge, and you can add me as feedstock's maintainer alongside with you.

I'm not sure what tests are failing for you. I pushed a few commits to get the CI environments working again to know for sure, and they are all passing their now after a few changes. They are very fragile unfortunately, but they are important because they always get the latest versions of numpy, scipy, etc - and that tells me if I'm out of date.

I need to check the CI. Maybe the problem is locally, in my PC. It's possible that I don't installed all the dependencies correct. ASAP I will submit a "fake" PR, just to see the tests' verbose output. However, I think the problem is probably local.

I have had precision trouble before but generally I prefer to keep a very tight tolerance on the tests. One molar volume method may be very similar to another, differing by 0.1% - it is important that the tests can tell the difference between the two, otherwise one may fail, the next be called, and the test will still pass.

I think you are right. Maybe I should forget about the tolerance issues if the test problem is local. A tighter tolerance is always better whenever it's possible.

You may be running tests without some of the dependencies, or with older versions of them? Sympy, CoolProp, and pint are required by some tests. I try to keep the test results so that they always pass on the latest versions of the dependencies.

Oh! I have to check if CoolProp is installed properly (maybe it didn't). Is it possible that pip is not getting all the dependencies? I saw the thermo's setup.py, and some packages (for tests) are not include. Unfortunately, the conda skeleton -- which mounts automatically conda recipes from pypi -- can't gather the CoolProp (and others) dependencies. I checked the Travis config file, you set the installation of the "prerequisites" (I don't if they really are prerequisites... is it just for tests?) outside with pip install numpydoc fluids coolprop pytest-cov coveralls pint. That's make sense, since the dev's environment is not the same as the user's environment. However, I think if one wish to test the libs, they must reproducible out-of-the-box, but this is only my opinion.

Caleb, I'm sorry if I'm being somewhat invasive, which is not my intention for sure. I really like your thermo lib, and I wish to improve it a little, if I can. Sorry if it bothers you in anyway.

Best regards, Diego.