Joss-reviews: [REVIEW]: Thermosteam: BioSTEAM's Premier Thermodynamic Engine

Created on 2 Nov 2020  ·  28Comments  ·  Source: openjournals/joss-reviews

Submitting author: @yoelcortes (Yoel Cortes-Pena)
Repository: https://github.com/BioSTEAMDevelopmentGroup/thermosteam
Version: 0.20.26
Editor: @kyleniemeyer
Reviewer: @decaluwe, @fwitte
Archive: Pending

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952"><img src="https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952/status.svg)](https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@decaluwe & @fwitte, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @kyleniemeyer know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist for @decaluwe

Conflict of interest

  • [x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@yoelcortes) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [ ] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • [ ] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [ ] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [ ] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @fwitte

Conflict of interest

  • [x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • [x] Repository: Is the source code for this software available at the repository url?
  • [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • [x] Contribution and authorship: Has the submitting author (@yoelcortes) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

  • [x] Installation: Does installation proceed as outlined in the documentation?
  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

  • [ ] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [ ] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [ ] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
Python review

Most helpful comment

From my perspective, the dependency requirement is satisfied for Python packages through including appropriately in setup.pu and/or requirements.txt (for example), as @fwitte mentioned

All 28 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @decaluwe, @fwitte it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

👋 @yoelcortes @decaluwe @fwitte This is where the review actually takes place. Thanks!

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1145/2833157.2833162 is OK
- 10.1021/acssuschemeng.9b07040 is OK
- 10.3390/pr8080904 is OK

MISSING DOIs

- None

INVALID DOIs

- None

Hi @kyleniemeyer - I made it all the way to step 2 🤓

The link https://github.com/openjournals/joss-reviews/invitations takes me to the following message:

Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account.

@whedon re-invite @decaluwe as reviewer

OK, the reviewer has been re-invited.

@decaluwe please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

@decaluwe does the link work now?

Yep—already started on the review. Thanks!

On Nov 2, 2020, at 1:53 PM, Kyle Niemeyer notifications@github.com wrote:

@decaluwe https://github.com/decaluwe does the link work now?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/2814#issuecomment-720718209, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEC7PF3W6TBJGH5BAFEBFTSN4L4FANCNFSM4THX5JMQ.

:wave: @decaluwe, please update us on how your review is going.

:wave: @fwitte, please update us on how your review is going.

I am currently having trouble installing the software. Have tried multiple pathways using pip, to no avail. Plan to try the GitHub source code, today...

I started into it. Will most likely complete the review this weekend or the following week. I am posting all software related issues at https://github.com/BioSTEAMDevelopmentGroup/thermosteam/ and will write a complete feedback here linking the respective issues.

I am currently having trouble installing the software. Have tried multiple pathways using pip, to no avail. Plan to try the GitHub source code, today...

@decaluwe Sorry to hear that! It might be a bug with the Python's new pip installer. But in the case it is not, I uploaded the latest version to PyPI (pip install thermosteam==0.21.7). Hope this fixes the issue.

Thanks!

Thanks - will give it a try!

Works now. 👏

2 quick questions:

  • @kyleniemeyer if they don't list all dependencies, but the installation is handled by an automated package manager, am I fine to check this item off?

Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

  • @yoelcortes I see that there is a test suite in the source code--are there instructions for running it? (I haven't tried yet, might be straightforward, but just thought I'd ask, while I'm here 😉 )

Thanks.

Hi @decaluwe, thanks for asking! If you have the github repository downloaded, go to your local thermosteam repository and run "pytest" in your cmd or terminal. Here are some instructions: https://biosteam.readthedocs.io/en/latest/CONTRIBUTING.html. Unfortunately I haven't added a note of this in Thermosteam's documentation yet.

I'd also like to note that continuous integration is built in on github, so that the package is installed in a virtual environment (on python 3.6, 3.7 and 3.8) and all the documentation examples and the test files are run with every push to the repository (the coverage is checked and updated as well). If you click on the red "X" mark, you can go to several links for details regarding tests of the last push:

image

As for listing dependencies, would the "requirements.txt" or "setup.py" file count? Note that if you downloaded the repository, you can use "pip install ." to let pip install thermosteam and its dependencies for you as well.

Hi @decaluwe and @kyleniemeyer,
according to https://joss.readthedocs.io/en/latest/review_criteria.html#installation-instructions I understood a list is not necessary with automatic installation of dependencies. Therefore I checked the box. Let me know if I did wrong.

@yoelcortes, maybe consider adding the 'extras_require' keyword to your setup.py, so developers will automatically have develop dependencies installed and do not need to install pytest, sphinx, ..., manually for testing/debugging on their respective virtual environment. I do not expect this to be implemented within the review, so feel free to do as you like :).
With

    extras_require={'dev': ['pytest', 'sphinx', 'sphinx_rtd_theme', ]}

in setup.py

pip install -e .[dev]

will install the extras.

Best regards
Francesco

Hi @fwitte, that is really helpful. I'll add it in. Thanks for sharing!

From my perspective, the dependency requirement is satisfied for Python packages through including appropriately in setup.pu and/or requirements.txt (for example), as @fwitte mentioned

Okay, thanks @kyleniemeyer . Since it was listed under the documentation section, I wasn't sure whether it needed to be explicitly listed, or if having it in requirements.txt was sufficient (since it is a PyPI-managed installation, it certainly seems sufficient to me).

FYI:

  1. @yoelcortes I get two test failures:
    ==================================== FAILURES================================
    _____________________________________________ test_wheatstraw ______________________________________________

    @pytest.mark.slow
    def test_wheatstraw():

      from biorefineries import wheatstraw as ws
    

    E ImportError: cannot import name 'wheatstraw' from 'biorefineries' (/Users/decaluwe/.local/lib/python3.7/site-packages/biorefineries/__init__.py)

../../../.local/lib/python3.7/site-packages/biorefineries/tests/test_biorefineries.py:74: ImportError
_____________________________________________ test_annimal_bedding ________________________________________

@pytest.mark.slow
def test_annimal_bedding():
  from biorefineries import animal_bedding as ab

E ImportError: cannot import name 'animal_bedding' from 'biorefineries' (/Users/decaluwe/.local/lib/python3.7/site-packages/biorefineries/__init__.py)

../../../.local/lib/python3.7/site-packages/biorefineries/tests/test_biorefineries.py:90: ImportError
============================ 2 failed, 153 passed in 12.11s =========================

  1. @kyleniemeyer, is the automated testing check box require that the tests pass, or simply that the test suite exists?

@decaluwe if you find that the tests are not passing on your system, then that would be something @yoelcortes should address, so you can leave it unchecked for now until the issue is addressed satisfactorily

@yoelcortes Notes on the software paper:

  1. Depending on one's definition of "a diverse, non-specialist audience," the intro could probably be written a little bit broader. For example, phrases like "unit operations" and even "chemical process" might be opaque for somebody in materials science or astronomy. I'd suggest something like: (i) end the first sentence right before the parenthetical, (ii) describe some common features of a "unit operation in a chemical process" (e.g. something along the lines of "fluid flows enter a reactor wherein chemical reactions occur, consuming or producing chemical species and thermal energy...") and then (iii) give some examples/categories of the specific types of properties needed for evaluating the design of a chemical process. It might add a sentence or two, but should be clearer for a non-expert. The distillation column example provides a good specific example, but something helping bridge the very general first sentence and this more specific example would be helpful.

  2. State of the field: There is no documenting of or comparison to current capabilities in the field. Would software like EES, CoolProp, RefProp, Cantera, or Chemkin be valid points of comparison? These packages might have similar functionality, but I do not know if they are "commonly used" in the field (which is what the review guidelines specifically ask), so I'd rely on your expertise, obviously. What approaches are commonly used in your field?

Not related to the paper, but a general compliment: the API documentation is very well done, and is well populated with examples, which I appreciated. Nice work!

@decaluwe, I made some minor changes in the repository that prevents the pytest failures (thanks for reporting). Also, your comments on the software paper are good points. I'll wait until Francesco can submit his comments too before I submit my edits.

Glad you liked the documentation. I was quite inspired by (and grateful of) other open source projects with excellent documentation. Thanks!

Hi @yoelcortes,
here are my general comments to the paper and your software (documentation).

Paper

  • The paper highlights the most important features of the software, but (as already written in the docs issue) I do miss the capability, that generic chemical reactions can be computed with your software. I think this is a very important feature of your software.
  • A statement regarding the main advantages compared to software in the field (like @decaluwe pointed out) would be very good to emphasize the value of thermosteam, too.
  • I am not a legal expert, is it necessary to include the disclaimer twice. Can it not be handled in a single sentence?
    Maybe something like "Any opinions, findings, and conclusions or recommendations expressed in this publication are those of the author and necessarily reflect the views of the funders and sponsors."?
  • Are these [@BioSTEAM][@Sanchis] meant to be two individual citations? You can connect them with [@BioSTEAM; @Sanchis]
  • Other than that, very good compact overview of the software.

Software (Documentation)

There is not much to say here, very well documented with an extensive tutorial on how to use the package. I like it very much!

Issues

Checked, if resolved.

Thanks @fwitte and @decaluwe!

@yoelcortes I think you are already working to address these issues, so please just report back here when you are done. Thanks!

Was this page helpful?
0 / 5 - 0 ratings