search

openjournals / jose-reviews

Reviews for the Journal of Open Source Education (JOSE)
http://jose.theoj.org
Creative Commons Zero v1.0 Universal
33 stars 4 forks source link

[REVIEW]: pendsim: Developing, Simulating, and Visualizing Feedback Controlled Inverted Pendulum Dynamics #168

Closed whedon closed 1 year ago

whedon commented 2 years ago

Submitting author: @rland93 (Mike Sutherland) Repository: https://github.com/EASEL-UCI/pendsim Version: v1.2 Editor: @moorepants Reviewer: @cooperrc, @repagh Archive: 10.5281/zenodo.7603937

:warning: JOSE reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSE is currently operating in a "reduced service mode".

Status

status

Status badge code:

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

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

@cooperrc & @repagh, 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/jose-reviews/invitations

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

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @repagh

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

moorepants commented 2 years ago

You are using a theme that has a dark mode. Many of the figures are unreadable, e.g.: image Maybe disable the dark mode as a quick fix?

moorepants commented 2 years ago

Tutorial figure on line 7 has misplaced axes labels: image

moorepants commented 2 years ago

The tutorial video on the last line does not fit on narrow screens: image

moorepants commented 2 years ago

The sys.append('../') lines should not be needed in your examples:

import sys
sys.path.append('../')
%matplotlib notebook
from pendsim import sim, controller, viz
import numpy as np
import matplotlib.pyplot as plt

If users install your package, they shouldn't have to do that.

moorepants commented 2 years ago

On the animation page there is some unusual directory changing:

import os, pathlib
os.chdir(pathlib.Path(globals()['_dh'][0]).parent)

This does not seem like good practice.

moorepants commented 2 years ago

On the linearization page there are some formatting errors, e.g.: image

moorepants commented 2 years ago

All of the cells are not executed on the PID page:

https://rland93.github.io/pendsim/PID.html

moorepants commented 2 years ago

@EASEL-UCI Can you address the various formatting and presentation issues I've mentioned here? This would bring the contents to publishable quality with JOSE. Once that is done, you can cut a new release.

@repagh and @cooperrc, I assume that the "community guidelines" checkbox was not checked by mistake. Can you check that box if you are satisfied with the statement in the README?

labarba commented 1 year ago

hi folks! 👋 Happy New Year... It looks like this is close to the finish line. We just need some final efforts on your part, @EASEL-UCI – have a look 👀

EASEL-UCI commented 1 year ago

@labarba Happy New Year. Thanks very much for the note. @moorepants Thank you for noticing these remaining issues and for the detailed comments. We've made the following changes and believe we've addressed each of the comments:

The new release v1.2 is up-to-date with these changes.

moorepants commented 1 year ago

@whedon set v1.2 as version

whedon commented 1 year ago

OK. v1.2 is the version.

moorepants commented 1 year ago

@whedon generate pdf

moorepants commented 1 year ago

@whedon check references

whedon commented 1 year ago

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

moorepants commented 1 year ago

@whedon check repository

whedon commented 1 year ago

Wordcount for paper.md is 1111

whedon commented 1 year ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.12 s (224.5 files/s, 323671.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                           9            270            580            939
Jupyter Notebook                 8              0          36076            572
Markdown                         2             62              0            184
YAML                             1              9             10             44
reStructuredText                 2             20             20             32
TeX                              1              2              0             29
DOS Batch                        1              8              1             26
make                             1              5              7             16
CSS                              1              1              0              8
TOML                             1              0              0              3
-------------------------------------------------------------------------------
SUM:                            27            377          36694           1853
-------------------------------------------------------------------------------

Statistical information for the repository '8ecd942a11012574e73252c5' was
gathered on 2023/02/01.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
EASEL Research Group             3             3              3            0.04
Mike Sutherland                 19          2585           2036           28.99
ms                              23          5399           4472           61.93
rland93                          9           997            445            9.05

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
EASEL Research Group          3          100.0          1.1                0.00
Mike Sutherland            1679           65.0         17.5                8.52
rland93                     107           10.7         14.6               25.23
whedon commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1515/9781400828739 is OK
- 10.1109/MCSE.2007.55 is OK

MISSING DOIs

- None

INVALID DOIs

- None
moorepants commented 1 year ago

@EASEL-UCI this still shows documentation for 1.1: https://rland93.github.io/pendsim/index.html Should I be looking elsewhere?

EASEL-UCI commented 1 year ago

@moorepants Yes, please see https://easel-uci.github.io/pendsim/index.html

moorepants commented 1 year ago

I've checked over everything again and I approve for publishing. @EASEL-UCI can you make the Zenodo archive of your version 1.2.0 and reply here with the DOI to that archive?

EASEL-UCI commented 1 year ago

@moorepants Thank you very much! Here is the Zenodo DOI: 10.5281/zenodo.7603937

moorepants commented 1 year ago

@whedon set 10.5281/zenodo.7603937 as archive

whedon commented 1 year ago

OK. 10.5281/zenodo.7603937 is the archive.

moorepants commented 1 year ago

@whedon recommend-accept

whedon commented 1 year ago 
Attempting dry run of processing paper acceptance...
whedon commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1515/9781400828739 is OK
- 10.1109/MCSE.2007.55 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 1 year ago

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

Check final proof :point_right: https://github.com/openjournals/jose-papers/pull/113

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/jose-papers/pull/113, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
moorepants commented 1 year ago

Hi @labarba, this looks ready from my perspective.

@repagh and @cooperrc, thank you very much for volunteering your time to review this submission.

@EASEL-UCI and @rland93, congrats on having this published!

EASEL-UCI commented 1 year ago

@moorepants, @repagh, and @cooperrc: Thank you very for your valuable feedback that greatly improved the software. We appreciate your time!

moorepants commented 1 year ago

@labarba, This paper is ready to be published.

labarba commented 1 year ago

I notice that both reviewers left the "Community guidelines" item of the checklist unchecked. But they both approved for publication in a later comment, @cooperrc here and @repagh here. I will proceed with publication, and mark those items as checked.

labarba commented 1 year ago

@whedon accept deposit=true

whedon commented 1 year ago 
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 1 year ago

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

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/jose-papers/pull/115
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/jose.00168
  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...

labarba commented 1 year ago

Congratulations, @rland93 and @EASEL-UCI, your JOSE paper is published! 🚀

Big thanks to our Editor: @moorepants, and to the Reviewers: @cooperrc, @repagh — your contributions make this adventure in publishing possible 🙏

whedon commented 1 year 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://jose.theoj.org/papers/10.21105/jose.00168/status.svg)](https://doi.org/10.21105/jose.00168)

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

reStructuredText:
.. image:: https://jose.theoj.org/papers/10.21105/jose.00168/status.svg
   :target: https://doi.org/10.21105/jose.00168

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Education 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: