[REVIEW]: Manim Slides: A Python package for presenting Manim content anywhere

whedon commented 1 year ago

Submitting author: !--author-handle-->@jeertmans@magsol<!--end-editor-- Reviewers: @behollister, @bryanwweber Archive: 10.5281/zenodo.8215167

Status

Status badge code:

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

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

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

Make sure you're logged in to your GitHub account
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 @magsol 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 @behollister

Conflict of interest

[x] As the reviewer I confirm that I have read the JOSE conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSE 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] Version: v4.13.2
[x] Authorship: Has the submitting author (@jeertmans) made substantial contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation? (and documentation is sufficient?)
[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 the need for this software 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?
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Tests: Are there automated tests or manual steps described so that the function 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

[x] Authors: Does the paper.md file include a list of authors with their affiliations?
[x] A statement of need: Do the authors clearly state the need for this software and who the target audience is?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @bryanwweber

Conflict of interest

[x] As the reviewer I confirm that I have read the JOSE conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSE 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] Version: v4.13.2
[x] Authorship: Has the submitting author (@jeertmans) made substantial contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation? (and documentation is sufficient?)
[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 the need for this software 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?
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] #207
[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

[x] Authors: Does the paper.md file include a list of authors with their affiliations?
[x] A statement of need: Do the authors clearly state the need for this software and who the target audience is?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

whedon commented 1 year ago

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

:warning: JOSE reduced service mode :warning:

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

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/jose-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:

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

watching

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

whedon commented 1 year ago

Wordcount for paper.md is 786

whedon commented 1 year ago

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.06 s (839.1 files/s, 86285.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          16            659            293           2531
Markdown                        19            284              0            688
YAML                            13             60             57            469
HTML                             1             68              5            218
TOML                             1             11              0             81
TeX                              1              5              0             48
DOS Batch                        1              8              1             26
Bourne Shell                     1             10              0             11
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                            54           1109            363           4081
-------------------------------------------------------------------------------

Statistical information for the repository 'db2d887a27b587ca985b537f' was
gathered on 2023/04/26.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Fairlight8                       2            50             10            0.84
Federico Galatolo               22           505             35            7.59
Jérome Eertmans                113          4623           1406           84.78
Linus Heck                       6            75             43            1.66
MikeGillotti                     1            15              4            0.27
Tomasz Dądela                    1            15            192            2.91
Wu Tingfeng                      1            53             34            1.22
Wucheng Zhang                    1             3              0            0.04
pre-commit-ci[bot]               4            24             22            0.65
yang-fighter                     1             1              1            0.03

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
Fairlight8                   48           96.0          0.7               10.42
Federico Galatolo            94           18.6          1.2                0.00
Jérome Eertmans            3273           70.8          4.5                5.16
Linus Heck                   16           21.3         22.0               12.50
MikeGillotti                 11           73.3          5.5                0.00
Tomasz Dądela                 9           60.0          4.3               33.33
Wu Tingfeng                  20           37.7          5.9               15.00
pre-commit-ci[bot]           11           45.8          5.5                0.00
yang-fighter                  1          100.0          3.2                0.00

whedon commented 1 year ago

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

OK DOIs

- None

MISSING DOIs

- None

INVALID DOIs

- None

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:

bryanwweber commented 1 year ago

@jeertmans Thanks for the paper and the package! I've created several issues in the repository related to my review:

jeertmans/manim-slides#168
jeertmans/manim-slides#169
jeertmans/manim-slides#170
jeertmans/manim-slides#171
jeertmans/manim-slides#172

Aside from those issues, the version that is tagged here for the paper is not the most current release. @magsol How should that version be updated? Thank you! I look forward to the responses.

jeertmans commented 1 year ago

Thanks for your review @bryanwweber! Indeed, the package has evolved quite a bit since the pre-review process was started. Most importantly, I have added a feature that converts Manim Slides presentations into PowerPoint files, which is pretty convenient for presenting in conferences for example.

I did not want to update the paper prior to the first review, but I can’t surely update the paper so it matches the latest version of Manim Slides.

For the rest of your review, I will address the comments directly in the PRs.

jeertmans commented 1 year ago

Hello @bryanwweber, I have opened a series of PRs to address each of your comments. I hope they answer your concerns correctly, and I hope open to any new suggestion you might have :-)

Already, thank you for your time!

magsol commented 1 year ago

@whedon set v4.12.0 as version

whedon commented 1 year ago

OK. v4.12.0 is the version.

bryanwweber commented 1 year ago

@jeertmans Thank you! I've left one comment on one of the PRs and otherwise they look good!

jeertmans commented 1 year ago

Thanks @bryanwweber! I've merged all the PRs and released the changes under v4.13.0, which also contains more debugging messages (explaining the bump in minor version).

Tell me if you need anything else to be done :-)

magsol commented 1 year ago

@whedon set v4.13.0 as version

whedon commented 1 year ago

OK. v4.13.0 is the version.

magsol commented 1 year ago

@behollister 👋 Hey Brad, wanted to check in and see if you needed anything for the review, or what kind of timetable you're looking at. Thanks!

whedon commented 1 year ago

:wave: @behollister, please update us on how your review is going (this is an automated reminder).

whedon commented 1 year ago

:wave: @bryanwweber, please update us on how your review is going (this is an automated reminder).

bryanwweber commented 1 year ago

My review is complete, thanks!

magsol commented 1 year ago

@jeertmans Wanted to keep you informed: we're looking to wrap up this review by early-to-mid next week. Sorry for the delay!

jeertmans commented 1 year ago

No problem at all @magsol :) Thanks for noticing me!

behollister commented 1 year ago

@jeertmans @magsol added issue related to review

https://github.com/jeertmans/manim-slides/issues/188

behollister commented 1 year ago

@jeertmans @magsol added issue related to review https://github.com/jeertmans/manim-slides/issues/193 @bryanwweber may have already verified but didn't see the resolution comment in the original issue: https://github.com/openjournals/jose-reviews/issues/207

concerns - Tests: Are there automated tests or manual steps described so that the function of the software can be verified?

and,

Functionality: Have the functional claims of the software been confirmed?

behollister commented 1 year ago

@jeertmans @magsol included additional issue related to review https://github.com/jeertmans/manim-slides/issues/194

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

behollister commented 1 year ago

@jeertmans Thank you for your recent clarifications. Yes, all of the issues I raised have been addressed.

magsol commented 1 year ago

@behollister Excellent, thank you so much!

magsol commented 1 year ago

@whedon generate pdf

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:

magsol commented 1 year ago

@whedon check references

whedon commented 1 year ago

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

OK DOIs

- None

MISSING DOIs

- None

INVALID DOIs

- None

magsol commented 1 year ago

@jeertmans Hi, now that both reviewers have recommended acceptance, I'm now starting the final review and acceptance procedures. I noticed your latest version hasn't been updated in a couple weeks (4.13.1 was the last). Would you mind 1) updating the version tag and archive if needed, and 2) reporting the archive DOI? Once I have these pieces of information, I'll proceed (as the PDF looks good!).

jeertmans commented 1 year ago

Hello @magsol, thanks for recalling about the DOI! There you are: https://zenodo.org/record/7971361.

Tell me if I need to do anything else :-)

magsol commented 1 year ago

@whedon set 10.5281/zenodo.7971361 as archive

whedon commented 1 year ago

OK. 10.5281/zenodo.7971361 is the archive.

magsol commented 1 year ago

@whedon set v4.13.2 as version

whedon commented 1 year ago

OK. v4.13.2 is the version.

magsol 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

- None

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/125

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

@whedon accept deposit=true

magsol commented 1 year ago

@jeertmans Thanks so much! Everything looks good--just waiting on EiCs to accept your paper!

jeertmans commented 1 year ago

Many thanks for review!

magsol commented 1 year ago

Hi @openjournals/jose-eics, just wanted to ping you on this one being ready for publication.

jeertmans commented 1 year ago

Hello @magsol, I don't think you mention worked here. Do you know why all publications are stalled on JOSE's papers repo?

magsol commented 1 year ago

@jeertmans I don't know, that's a good question. I'm so sorry for the delay. Let me ping the editors directly and hopefully we'll get something moving here.

labarba commented 1 year ago

Hi everybody! 👋 — It's me who's backlogged due to travel, keynote, more travel, and email overload 😬 Thank you for your patience.

For some reason, the archive does not show on the top of the issue...

But I see that @magsol ran the command to add it above.

I will try to add it again with our upgraded editorial bot...

labarba commented 1 year ago

@editorialbot set 10.5281/zenodo.7971361 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.7971361

labarba commented 1 year ago

have a look at this tiny fix: https://github.com/labarba/manim-slides/pull/1

jeertmans commented 1 year ago

Thanks for your suggestion @labarba, this is fixed :-)

As the v4.15.0 was released, and the paper is still not published, maybe it would be possible to update the version to v4.15.0, and the Zenodo to 10.5281/zenodo.8215167?

labarba commented 1 year ago

Ah. Do you have the GitHub Zenodo integration set up to update the archive automatically with each release? We do like to ask authors to edit the metadata of the archive so that the title and author list match the paper. (The auto-archives pull as authors all committers to the repo and use the repo name as title.) Could you do that change for the archive we will attach to the JOSE paper?

openjournals / jose-reviews