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 badge code:
HTML: <a href="https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952"><img src="https://joss.theoj.org/papers/e04f122cd968362dec0456d2a9161952/status.svg"></a>
Markdown: [](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.)
@decaluwe & @fwitte, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
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 ✨
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:
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:
Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
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:
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:
@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 =========================
@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:
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.
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).
[@BioSTEAM][@Sanchis]
meant to be two individual citations? You can connect them with [@BioSTEAM; @Sanchis]
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!
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!
Most helpful comment
From my perspective, the dependency requirement is satisfied for Python packages through including appropriately in
setup.pu
and/orrequirements.txt
(for example), as @fwitte mentioned