CalebBell / thermo

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

Linting : Proof-of-concept stylistic refactorings #8

Closed apthorpe closed 3 years ago

apthorpe commented 7 years ago

This replaces the previous closed pull request, pulling from a different branch (apthorpe:linting instead of apthorpe:master). acentric.py and activity.py have been modified based on previous discussion and results of analysis with updated versions of pylint and flake8. Additionally, pylintrc has been added to the project to avoid the use of local pylint directives in the source.

CalebBell commented 7 years ago

Hello again Bob, As a reminder, the issues outstanding are:

About the doctests, for instance, you changed: {'H': 0.03639798802478244, 'C': 0.7228692758981262, 'O': 0.24073273607709128} to be

{'H': 0.03639798802478244, 'C': 0.7228692758981262,
'O': 0.24073273607709128} 

But the doctests will fail because of that change; they do not support handling multiple lines of output. All the output has to be on line line.

Another thing about doctests - you're changing some of them from >>> thermal_conductivity_Magomedov(293., 1E6, [.25], ['7758-94-3'], k_w=0.59827) to be:

>>> thermal_conductivity_Magomedov(293., 1E6, [.25], ['7758-94-3'],
...                                k_w=0.59827) 

which I am happy for except all the extra spaces between the "..." and the "k_w=0.59827". So far as I know projects do not use this syntax, as it's better for the user to be able to copy the example and run it with as little work as possible. The preferred form is:

>>> thermal_conductivity_Magomedov(293., 1E6, [.25], ['7758-94-3'],
... k_w=0.59827) 

You've done a great job of working on the documentation for some undocumented functions! Thank you very much for that. I hope to supplement them even more. If you have any questions about the requested changes, please feel free to ask!

Sincerely, Caleb

alexchandel commented 4 years ago

@CalebBell The space-indented form of:

>>> thermal_conductivity_Magomedov(293., 1E6, [.25], ['7758-94-3'],
...                                k_w=0.59827) 

is common and recommended by Python's style guide. The "preferred form" you requested is forbidden by the same.

CalebBell commented 3 years ago

Hi Bob, I'm sorry this never went anywhere. At this point in my life not interested in improving the style, there are way too many larger and more important issues.

Pylint is extremely opinionated, and everywhere I see it used, an awful of of its rules are disabled. Only the errors category interests me at this point. I do have a .pylintrc file that is used for that purpose in thermo now but I definitely am not running it regularly or over all files. Sometimes it misses things too, claims things are errors when they aren't.

I hope all is well in your life!

@alexchandel I don't think everything in pep8 is suitable for every project; this project follows it loosely. I am also extremely done with 80 character lines being forced, I think it wastes millions of people-hours every year. If I had to put a space on either side of every multiply sign, thermo would be much harder for me to follow.

Sincerely, Caleb