openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
712 stars 38 forks source link

[REVIEW]: PyRolL - An Extensible OpenSource Framework for Rolling Simulation #6200

Closed editorialbot closed 8 months ago

editorialbot commented 8 months ago

Submitting author: !--author-handle-->@axtimhaus<!--end-author-handle-- (Max Weiner) Repository: https://github.com/pyroll-project/pyroll-core Branch with paper.md (empty if default branch): joss Version: v2.1.3 Editor: !--editor-->@philipcardiff<!--end-editor-- Reviewers: @philipcardiff, @rboman Archive: 10.5281/zenodo.10495163

Status

status

Status badge code:

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

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

@philipcardiff & @rboman, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @philipcardiff 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

Checklists

📝 Checklist for @philipcardiff

📝 Checklist for @rboman

editorialbot commented 8 months ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

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

@editorialbot generate pdf
editorialbot commented 8 months ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.11 s (1058.8 files/s, 94871.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                         102           1985            858           5149
TeX                              1             94              0            993
Markdown                         8            111              0            292
TOML                             2             12              1             78
YAML                             1              1              4             18
Jupyter Notebook                 1              0            706              2
-------------------------------------------------------------------------------
SUM:                           115           2203           1569           6532
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 8 months ago

Wordcount for paper.md is 1270

philipcardiff commented 8 months ago

Review checklist for @philipcardiff

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

editorialbot commented 8 months ago

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

editorialbot commented 8 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1098/rspa.1972.0025 is OK
- 10.12688/f1000research.23224.2 is OK
- 10.1016/j.commatsci.2005.01.009 is OK
- 10.1002/srin.200806353 is OK
- 10.2355/isijinternational.46.1458 is OK
- 10.1016/S1005-8850(08)60065-1 is OK
- 10.1179/030192304225011016 is OK
- 10.1002/srin.200506068 is OK
- 10.1002/srin.196705760 is OK
- 10.1016/j.jmatprotec.2004.12.007 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.rcim.2014.04.006 is OK
- 10.1016/j.commatsci.2018.07.043 is OK
- 10.1002/nme.1328 is OK
- 10.1002/srin.200706280 is OK
- 10.1007/s11665-008-9289-2 is OK
- 10.1016/S0924-0136(00)00734-2 is OK
- 10.1016/S0921-5093(01)01964-5 is OK
- 10.1016/S0924-0136(02)00347-3 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.42.868 is OK
- 10.1016/0924-0136(96)02307-2 is OK
- 10.1016/j.ijmachtools.2005.07.023 is OK
- 10.25518/esaform21.3987 is OK
- 10.5281/ZENODO.6053272 is OK
- 10.1002/srin.198801607 is OK
- 10.1051/metal/193936060257 is OK
- 10.1016/0890-6955(92)90022-9 is OK
- 10.1016/j.proeng.2015.12.038 is OK
- 10.1243/PIME_PROC_1961_175_043_02 is OK
- 10.1016/j.proeng.2014.09.149 is OK
- 10.1016/j.jmatprotec.2007.04.124 is OK
- 10.1007/978-3-322-88346-9 is OK
- 10.2355/isijinternational.40.65 is OK

MISSING DOIs

- 10.25518/esaform21.3987 may be a valid DOI for title: A Computational Method for Pass Design of the Four-Roll Rolling Process Forsizing of Round Sections

INVALID DOIs

- None
philipcardiff commented 8 months ago

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

OK DOIs

  • 10.1098/rspa.1972.0025 is OK
  • 10.12688/f1000research.23224.2 is OK
  • 10.1016/j.commatsci.2005.01.009 is OK
  • 10.1002/srin.200806353 is OK
  • 10.2355/isijinternational.46.1458 is OK
  • 10.1016/S1005-8850(08)60065-1 is OK
  • 10.1179/030192304225011016 is OK
  • 10.1002/srin.200506068 is OK
  • 10.1002/srin.196705760 is OK
  • 10.1016/j.jmatprotec.2004.12.007 is OK
  • 10.1038/s41586-020-2649-2 is OK
  • 10.1016/j.rcim.2014.04.006 is OK
  • 10.1016/j.commatsci.2018.07.043 is OK
  • 10.1002/nme.1328 is OK
  • 10.1002/srin.200706280 is OK
  • 10.1007/s11665-008-9289-2 is OK
  • 10.1016/S0924-0136(00)00734-2 is OK
  • 10.1016/S0921-5093(01)01964-5 is OK
  • 10.1016/S0924-0136(02)00347-3 is OK
  • 10.2355/isijinternational.35.1100 is OK
  • 10.2355/isijinternational.35.1100 is OK
  • 10.2355/isijinternational.42.868 is OK
  • 10.1016/0924-0136(96)02307-2 is OK
  • 10.1016/j.ijmachtools.2005.07.023 is OK
  • 10.25518/esaform21.3987 is OK
  • 10.5281/ZENODO.6053272 is OK
  • 10.1002/srin.198801607 is OK
  • 10.1051/metal/193936060257 is OK
  • 10.1016/0890-6955(92)90022-9 is OK
  • 10.1016/j.proeng.2015.12.038 is OK
  • 10.1243/PIME_PROC_1961_175_043_02 is OK
  • 10.1016/j.proeng.2014.09.149 is OK
  • 10.1016/j.jmatprotec.2007.04.124 is OK
  • 10.1007/978-3-322-88346-9 is OK
  • 10.2355/isijinternational.40.65 is OK

MISSING DOIs

  • 10.25518/esaform21.3987 may be a valid DOI for title: A Computational Method for Pass Design of the Four-Roll Rolling Process Forsizing of Round Sections

INVALID DOIs

  • None

Hi @axtimhaus, can you check this possible missing DOI? Thanks.

rboman commented 8 months ago

Review checklist for @rboman

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

axtimhaus commented 8 months ago

@philipcardiff I have updated the DOI.

philipcardiff commented 8 months ago

@editorialbot check references

editorialbot commented 8 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1098/rspa.1972.0025 is OK
- 10.12688/f1000research.23224.2 is OK
- 10.1016/j.commatsci.2005.01.009 is OK
- 10.1002/srin.200806353 is OK
- 10.2355/isijinternational.46.1458 is OK
- 10.1016/S1005-8850(08)60065-1 is OK
- 10.1179/030192304225011016 is OK
- 10.1002/srin.200506068 is OK
- 10.1002/srin.196705760 is OK
- 10.1016/j.jmatprotec.2004.12.007 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.rcim.2014.04.006 is OK
- 10.1016/j.commatsci.2018.07.043 is OK
- 10.1002/nme.1328 is OK
- 10.1002/srin.200706280 is OK
- 10.1007/s11665-008-9289-2 is OK
- 10.1016/S0924-0136(00)00734-2 is OK
- 10.1016/S0921-5093(01)01964-5 is OK
- 10.1016/S0924-0136(02)00347-3 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.42.868 is OK
- 10.1016/0924-0136(96)02307-2 is OK
- 10.1016/j.ijmachtools.2005.07.023 is OK
- 10.25518/esaform21.3987 is OK
- 10.25518/esaform21.3987 is OK
- 10.5281/ZENODO.6053272 is OK
- 10.1002/srin.198801607 is OK
- 10.1051/metal/193936060257 is OK
- 10.1016/0890-6955(92)90022-9 is OK
- 10.1016/j.proeng.2015.12.038 is OK
- 10.1243/PIME_PROC_1961_175_043_02 is OK
- 10.1016/j.proeng.2014.09.149 is OK
- 10.1016/j.jmatprotec.2007.04.124 is OK
- 10.1007/978-3-322-88346-9 is OK
- 10.2355/isijinternational.40.65 is OK

MISSING DOIs

- None

INVALID DOIs

- None
philipcardiff commented 8 months ago

FYI: https://github.com/pyroll-project/pyroll-docs/issues/8

philipcardiff commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

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

rboman commented 8 months ago

Hi, I have posted my review as a MR (pyroll-docs) and an issue (pyroll-core).

Globally, I think that this piece of software is very interesting and deserves a paper in JOSS. As the authors say in their document, this kind of numerical model is usually kept private in companies. Consequently making it open-source is a real gift for researchers (and companies). It is also very modular and built in a modern way. The code is clean. I also like the plugin system with hooks. I haven't had enough time to understand how it internally works but I am sure I'll have a closer look in the future for my own projects.

I have been very happy with the documentation although I saw that there is an issue which says that it should be completely rewritten. PyRolL is very easy to install with PyPI and it is also straightforward to make a quick simulation (I like the "create-input-py" command of the pyroll executable which gives a first basic test for the new user). The input and outputs are easily understandable and the generated HTML report is very convenient to analyse the results. I have also played with the example Jupyter notebooks (pyroll-examples). Everything works as expected.

Therefore, I have no "major changes" to request. I will be happy to accept the paper as soon as my comments in the issue have been resolved.

philipcardiff commented 8 months ago

Thanks @rboman. My review is also in progress, and I agree with your comments and experiences.

philipcardiff commented 8 months ago

For future reference: https://github.com/pyroll-project/pyroll-core/issues/158#issuecomment-1889612249

axtimhaus commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

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

philipcardiff commented 8 months ago

Hi @axtimhaus, thank you again for this. My review is complete. I agree with @rboman 's comments, and I recommend it is accepted. It is a very nice toolbox; thank you for sharing. I, too, hope to try it out in some of my projects.

Please check the article again for any final changes. Once done, please issue a new tagged release of the software (if changed) and archive it (e.g. on Zenodo, figshare, or others). Please then post the version number and archive DOI here.

axtimhaus commented 8 months ago

The latest release is 2.1.3 archived at https://doi.org/10.5281/zenodo.10495163

Thank you both for the review. It was pleasant and productive, wish it could be always like that.

philipcardiff commented 8 months ago

@editorialbot set v2.1.3 as version

editorialbot commented 8 months ago

Done! version is now v2.1.3

philipcardiff commented 8 months ago

@editorialbot set 10.5281/zenodo.10495163 as archive

editorialbot commented 8 months ago

Done! archive is now 10.5281/zenodo.10495163

philipcardiff commented 8 months ago

@editorialbot recommend-accept

editorialbot commented 8 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 8 months ago

:wave: @openjournals/pe-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4919, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

philipcardiff commented 8 months ago

Hi @kyleniemeyer, this paper is ready for processing.

editorialbot commented 8 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1098/rspa.1972.0025 is OK
- 10.12688/f1000research.23224.2 is OK
- 10.1016/j.commatsci.2005.01.009 is OK
- 10.1002/srin.200806353 is OK
- 10.2355/isijinternational.46.1458 is OK
- 10.1016/S1005-8850(08)60065-1 is OK
- 10.1179/030192304225011016 is OK
- 10.1002/srin.200506068 is OK
- 10.1002/srin.196705760 is OK
- 10.1016/j.jmatprotec.2004.12.007 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.rcim.2014.04.006 is OK
- 10.1016/j.commatsci.2018.07.043 is OK
- 10.1002/nme.1328 is OK
- 10.1002/srin.200706280 is OK
- 10.1007/s11665-008-9289-2 is OK
- 10.1016/S0924-0136(00)00734-2 is OK
- 10.1016/S0921-5093(01)01964-5 is OK
- 10.1016/S0924-0136(02)00347-3 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.42.868 is OK
- 10.1016/0924-0136(96)02307-2 is OK
- 10.1016/j.ijmachtools.2005.07.023 is OK
- 10.25518/esaform21.3987 is OK
- 10.25518/esaform21.3987 is OK
- 10.5281/ZENODO.6053272 is OK
- 10.1002/srin.198801607 is OK
- 10.1051/metal/193936060257 is OK
- 10.1016/0890-6955(92)90022-9 is OK
- 10.1016/j.proeng.2015.12.038 is OK
- 10.1243/PIME_PROC_1961_175_043_02 is OK
- 10.1016/j.proeng.2014.09.149 is OK
- 10.1016/j.jmatprotec.2007.04.124 is OK
- 10.1007/978-3-322-88346-9 is OK
- 10.2355/isijinternational.40.65 is OK

MISSING DOIs

- None

INVALID DOIs

- None
ChRen95 commented 8 months ago

Just noticed, that we forgot to mention one valuable contributor which we want to thank. I just updated the paper.md.

philipcardiff commented 8 months ago

@editorialbot generate pdf

philipcardiff commented 8 months ago

@editorialbot recommend-accept

editorialbot commented 8 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 8 months ago

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

editorialbot commented 8 months ago

:warning: Error preparing paper acceptance.

editorialbot commented 8 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1098/rspa.1972.0025 is OK
- 10.12688/f1000research.23224.2 is OK
- 10.1016/j.commatsci.2005.01.009 is OK
- 10.1002/srin.200806353 is OK
- 10.2355/isijinternational.46.1458 is OK
- 10.1016/S1005-8850(08)60065-1 is OK
- 10.1179/030192304225011016 is OK
- 10.1002/srin.200506068 is OK
- 10.1002/srin.196705760 is OK
- 10.1016/j.jmatprotec.2004.12.007 is OK
- 10.1038/s41586-020-2649-2 is OK
- 10.1016/j.rcim.2014.04.006 is OK
- 10.1016/j.commatsci.2018.07.043 is OK
- 10.1002/nme.1328 is OK
- 10.1002/srin.200706280 is OK
- 10.1007/s11665-008-9289-2 is OK
- 10.1016/S0924-0136(00)00734-2 is OK
- 10.1016/S0921-5093(01)01964-5 is OK
- 10.1016/S0924-0136(02)00347-3 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.35.1100 is OK
- 10.2355/isijinternational.42.868 is OK
- 10.1016/0924-0136(96)02307-2 is OK
- 10.1016/j.ijmachtools.2005.07.023 is OK
- 10.25518/esaform21.3987 is OK
- 10.25518/esaform21.3987 is OK
- 10.5281/ZENODO.6053272 is OK
- 10.1002/srin.198801607 is OK
- 10.1051/metal/193936060257 is OK
- 10.1016/0890-6955(92)90022-9 is OK
- 10.1016/j.proeng.2015.12.038 is OK
- 10.1243/PIME_PROC_1961_175_043_02 is OK
- 10.1016/j.proeng.2014.09.149 is OK
- 10.1016/j.jmatprotec.2007.04.124 is OK
- 10.1007/978-3-322-88346-9 is OK
- 10.2355/isijinternational.40.65 is OK

MISSING DOIs

- None

INVALID DOIs

- None
kyleniemeyer commented 8 months ago

Hi @axtimhaus, I noticed that the author lists differ between the paper and Zenodo archive. We typically ask that those match, unless for a particular reason. Can you fix that, or let us know a reason for the difference (if intentional)?

Also, I made some minor edits to the paper in https://github.com/pyroll-project/pyroll-core/pull/164. Can you review and merge?

kyleniemeyer commented 8 months ago

@editorialbot generate pdf

editorialbot commented 8 months ago

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

kyleniemeyer commented 8 months ago

@axtimhaus the paper edits are good; can you address or resolve the Zenodo archive author list?

axtimhaus commented 8 months ago

I have updated the Zenodo record. These were automatically filled from the Python metadata.

kyleniemeyer commented 8 months ago

@editorialbot accept

editorialbot commented 8 months ago
Doing it live! Attempting automated processing of paper acceptance...
editorialbot commented 8 months ago

Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository.

If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file.

You can copy the contents for your CITATION.cff file here:

CITATION.cff

``` cff-version: "1.2.0" authors: - email: max.weiner@imf.tu-freiberg.de family-names: Weiner given-names: Max orcid: "https://orcid.org/0000-0002-8232-6877" - email: christoph.renzing@imf.tu-freiberg.de family-names: Renzing given-names: Christoph orcid: "https://orcid.org/0000-0002-9250-4358" - email: max.stirl@imf.tu-freiberg.de family-names: Stirl given-names: Max orcid: "https://orcid.org/0000-0003-3484-9849" - email: matthias.schmidtchen@imf.tu-freiberg.de family-names: Schmidtchen given-names: Matthias orcid: "https://orcid.org/0000-0002-2064-4124" - email: ulrich.prahl@imf.tu-freiberg.de family-names: Prahl given-names: Ulrich orcid: "https://orcid.org/0000-0001-6978-5721" doi: 10.5281/zenodo.10495163 message: If you use this software, please cite our article in the Journal of Open Source Software. preferred-citation: authors: - email: max.weiner@imf.tu-freiberg.de family-names: Weiner given-names: Max orcid: "https://orcid.org/0000-0002-8232-6877" - email: christoph.renzing@imf.tu-freiberg.de family-names: Renzing given-names: Christoph orcid: "https://orcid.org/0000-0002-9250-4358" - email: max.stirl@imf.tu-freiberg.de family-names: Stirl given-names: Max orcid: "https://orcid.org/0000-0003-3484-9849" - email: matthias.schmidtchen@imf.tu-freiberg.de family-names: Schmidtchen given-names: Matthias orcid: "https://orcid.org/0000-0002-2064-4124" - email: ulrich.prahl@imf.tu-freiberg.de family-names: Prahl given-names: Ulrich orcid: "https://orcid.org/0000-0001-6978-5721" date-published: 2024-01-23 doi: 10.21105/joss.06200 issn: 2475-9066 issue: 93 journal: Journal of Open Source Software publisher: name: Open Journals start: 6200 title: PyRolL - An Extensible OpenSource Framework for Rolling Simulation type: article url: "https://joss.theoj.org/papers/10.21105/joss.06200" volume: 9 title: PyRolL - An Extensible OpenSource Framework for Rolling Simulation ```

If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation.

Find more information on .cff files here and here.

editorialbot commented 8 months ago

🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘

editorialbot commented 8 months ago

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/4943
  2. Wait five minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.06200
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

kyleniemeyer commented 8 months ago

Congratulations @axtimhaus on your article's publication in JOSS! Please consider signing up as a reviewer if you haven't already.

Many thanks to @rboman for reviewing this, and @philipcardiff for editing.

editorialbot commented 8 months ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.06200/status.svg)](https://doi.org/10.21105/joss.06200)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.06200">
  <img src="https://joss.theoj.org/papers/10.21105/joss.06200/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.06200/status.svg
   :target: https://doi.org/10.21105/joss.06200

This is how it will look in your documentation:

DOI

We need your help!

The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following: