[JOSS Review] Inconsistency between documentation and the PyPI source

[yuzie007]

(This is a part of my review for JOSS openjournals/joss-reviews#7365. cc: @srmnitc)

Dear @jaclark5 and other authors of DESPASITO,

I am Yuji Ikeda @yuzie007 and reviewing DESPASITO for JOSS.
Thank you for writing a wonderful code and making it open-source.

Following the JOSS guideline, I am first checking the consistency with the documentation and the actual code in the PyPI source code. Could you check the points below and, when necessary, update the repo?

I will also (1) test other examples and (2) read the paper submitted to JOSS. The feedback for these will be reported in other issues.

• Functionality – Installation: In README.md and getting_started.rst, the available Python version is written up to 3.8, which has already been its end-of-life. The authors should update check and update the Python versions that are available for DESPASITO and not yet their end-of-life.

• Functionality – Installation: In getting_started.rst, the authors write "Option 3: Install locally with python". This looks for me redundant to "Option 2: Install locally with pip".
  I am not sure if we need to recommend users Option 3. In the modern Python they use pyproject.toml rather than setup.py, and probably Option 3 does not work with pyproject.toml.

• Documentation – Automated tests: In the present GitHub Actions workflow, the automated tests are done only up to Python 3.10. Likely the authors can update this to check up to which latest Python versions are available for DESPASITO.

• Documentation – Example usage: While in settingup.rst the output file name of the first example is written as despasito_out.txt, I actually obtainedout_vapor.txt. Please cross check and, if reasonable, revise this.

• Documentation – Example usage: In settingup.rst, the example usage as a python package is given as hexane_heptane_test.txt. However, the content is inconsistent with despasito/examples/saft_gamma_mie/hexane_heptane_activity_coeff/hexane_heptane_test.py. Further, both the script shows error in the directory despasito/examples/saft_gamma_mie/hexane_heptane_activity_coeff as

Traceback (most recent call last):
  File "/Users/ikeda/miniforge3/envs/myenv/lib/python3.11/site-packages/despasito/thermodynamics/__init__.py", line 61, in thermo
    output_dict = func(Eos, **kwargs)
                  ^^^^^^^^^^^^^^^^^^^
  File "/Users/ikeda/miniforge3/envs/myenv/lib/python3.11/site-packages/despasito/thermodynamics/calculation_types.py", line 1043, in vapor_properties
    raise ValueError("Ambiguous mixture composition. Define yilist")
ValueError: Ambiguous mixture composition. Define yilist

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/ikeda/codes/despasito/despasito/examples/saft_gamma_mie/hexane_heptane_activity_coeff/test.py", line 18, in <module>
    output = thermo.thermo(
             ^^^^^^^^^^^^^^
  File "/Users/ikeda/miniforge3/envs/myenv/lib/python3.11/site-packages/despasito/thermodynamics/__init__.py", line 63, in thermo
    raise TypeError("The calculation type, '{}', failed".format(calculation_type))
TypeError: The calculation type, 'vapor_properties', failed

and

Traceback (most recent call last):
  File "/Users/ikeda/codes/despasito/despasito/examples/saft_gamma_mie/hexane_heptane_activity_coeff/hexane_heptane_test.py", line 8, in <module>
    despasito.initiate_logger(console=True, verbose=10)
  File "/Users/ikeda/miniforge3/envs/myenv/lib/python3.11/site-packages/despasito/__init__.py", line 95, in initiate_logger
    handler_logfile.close()
    ^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'close'

• Documentation – Example usage: In startfitting.rst, it is written that the input_fit.json for methanol can be found in the examples directory. However, I could only found it fro another system in examples/saft_gamma_mie/butane_solubility. (I could run input_fit.json in this directory.)

[JacksonBurns]

Hi I am the other reviewer. I also noticed the inconsistent listing of support Python versions between the README and the CI, but also in the setup.py file, which allows any modern version:

    python_requires=">=3.6",

[jaclark5]

@yuzie007 Thank you for your comments and pointing out the inconsistencies. I've addressed them in #39 if you'd like to view them. Notice that the docs fail due to the update in dependencies required that have been included in PR 38.

Although I updated the tested versions of python and the documentation to reflect that, this PR will likely have a few more commits if there are issues with later versions of python. I will comment again when all tests except for readthedocs are clear.

[jaclark5]

@JacksonBurns I've addressed the inconsistencies between the paper, README, and CI testing. I will keep the setup.py file as is so that installation won't automatically fail with future releases of python, quite possibly without cause.

[jaclark5]

@yuzie007 all tests have passed for python 3.10, 3.11, and 3.12. python 3.13 is not yet supported by the condo-incubator used in GitHub actions.

[yuzie007]

Thank you so much for #39 and information. Regarding Python 3.13, if it cannot be tested just due to the GitHub action issue, I would feel it is OK to also include Python 3.13 also in the support, if it can pass all tests, e.g., in a local environment.

After #38 and #39 are merged, I would like to try remaining tests, and then I think I will finish my JOSS review.

[jaclark5]

Both #38 and #39 have been merged. #40 contains changes from the other reviewer.