JOSS Review: Documentation

OpenTerrace / openterrace-python

OpenTerrace: A fast, flexible and extendable Python framework for packed bed thermal energy storage simulations

https://openterrace.github.io/openterrace-python/

GNU General Public License v3.0

19 stars 2 forks source link

JOSS Review: Documentation #168

Closed baperry2 closed 3 months ago

baperry2 commented 4 months ago

Statement of need:

[x] Adding a bit more detail on the uses of the code on the landing page of the documentation would be helpful

Installation instructions:

[x] See installation comments in #167

Example usage:

[x] Sample results are missing for Tutorial 7
[x] It would be helpful to have a bit more detail on what each tutorial is, as well as a sentence or two describing the results in addition to the figures.

Automated tests:

[x] There are automated tests that run through a github workflow when PRs are merged. However, this is largely undocumented. Adding a bit to the documentation about these tests and how users/potential developers can run them would be helpful.

jakobhaervig commented 4 months ago

Thanks for these comments.

I added a bit more information on the documentation landing page while still trying to keep it simple.
All tutorials have been updated with a description in the header.
I added some information in the header of each automated test.

See https://github.com/OpenTerrace/openterrace-python/commit/b00f8cb88ab1cd88fa0c246a06a8af77017cd023

baperry2 commented 4 months ago

Thanks. Can you link to the PRs or commits where the changes were made to streamline things for me to complete the review?

baperry2 commented 4 months ago

Thanks for the link to the commit.

Sample results are still missing for tutorial 7: https://openterrace.github.io/openterrace-python/tutorials/tutorial_7/

jakobhaervig commented 3 months ago

Results for tutorial 7 were added here https://github.com/OpenTerrace/openterrace-python/commit/8e42b2f0308d9464a6cba1e793e2ac98e41a3402. Thanks!

baperry2 commented 3 months ago

That commit doesn't appear to have been merged into your main branch (the online docs linked above still don't show the figure). But that's minor and will presumably get merged eventually so I'll close this issue.