openjournals / joss-reviews

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

[REVIEW]: colorspace: A Python Toolbox for Manipulating and Assessing Colors and Palettes #7120

Closed editorialbot closed 2 weeks ago

editorialbot commented 2 months ago

Submitting author: !--author-handle-->@retostauffer<!--end-author-handle-- (Reto Stauffer) Repository: https://github.com/retostauffer/python-colorspace Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@jromanowska<!--end-editor-- Reviewers: @hollowscene, @dmreagan Archive: 10.5281/zenodo.14004295

Status

status

Status badge code:

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

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

@hollowscene & @dmreagan, 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 @jromanowska 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 @hollowscene

📝 Checklist for @dmreagan

editorialbot commented 2 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 2 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.23 s (360.1 files/s, 108731.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          55           4134           7429           9260
CSV                              1              0              0           1058
TeX                              2            129             15            986
Markdown                         4            157              0            459
YAML                             6             54             79            344
JSON                             6             13              0            318
SVG                              3              2              2            233
Sass                             2             17             14            134
make                             1             22             32             65
R                                2             31             18             46
INI                              1              1              0              7
-------------------------------------------------------------------------------
SUM:                            83           4560           7589          12910
-------------------------------------------------------------------------------

Commit count by author:

   499  Reto Stauffer
    54  =
    34  Achim Zeileis
     6  github-actions[bot]
editorialbot commented 2 months ago

Paper file info:

📄 Wordcount for paper.md is 1582

✅ The paper includes a Statement of need section

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

OK DOIs

- 10.1038/s41467-020-19160-7 is OK
- 10.18637/jss.v096.i01 is OK
- 10.1109/tvcg.2009.113 is OK
- 10.1109/mcse.2007.55 is OK
- 10.21105/joss.03021 is OK
- 10.5281/zenodo.10957263 is OK
- 10.5281/zenodo.12514964 is OK
- 10.1038/s41586-020-2649-2 is OK

MISSING DOIs

- 10.1016/0306-4573(91)90096-5 may be a valid DOI for title: Envisioning Information
- No DOI given, and none found for title: Color Use Guidelines for Data Representation
- No DOI given, and none found for title: Color
- No DOI given, and none found for title: Fundamentals of Data Visualization
- No DOI given, and none found for title: What’s New in Matplotlib 2.0 (Jan 17, 2017), Chang...
- No DOI given, and none found for title: Colour for Presentation Graphics
- No DOI given, and none found for title: Colormap
- No DOI given, and none found for title: Colormaps
- No DOI given, and none found for title: palettable: Color Palettes for Python
- No DOI given, and none found for title: cmcrameri: Python Wrapper around Fabio Crameri’s P...

INVALID DOIs

- None
editorialbot commented 2 months ago

License info:

🟡 License found: Other (Check here for OSI approval)

editorialbot commented 2 months ago

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

jromanowska commented 2 months ago

👋🏼 @retostauffer @hollowscene @dmreagan this is the review thread for the paper. All of our communications will happen here from now on.

As a reviewer, the first step is to create a checklist for your review by entering

@editorialbot generate my checklist

as the top of a new comment in this thread.

These checklists contain the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. The first comment in this thread also contains links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#7120 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

I encourage you to over-communicate and let everyone know that you're on the task every now and then, instead of waiting several weeks to collect all your comments, questions, or suggestions.

Please feel free to ping me (@jromanowska) if you have any questions/concerns.

hollowscene commented 2 months ago

Review checklist for @hollowscene

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

hollowscene commented 2 months ago

Hey everyone, just thought I'd make an early start to some of the housekeeping tickboxes. As a heads up - I'll most likely hold off on actually installing and playing around with the package until I've got a bit more time this weekend.

I hate to be nitpicky but just for the statement of need tickboxes, I feel like I'm not getting an explicit designation of who the target audience is. I definitely acknowledge it does feel a bit redundant for this package considering how ubiquitous its usage should be, and it sort of is implied within the current statement of need. But just wanted to double check this before ticking it off as I'm not entirely sure how strict we need to be!

jromanowska commented 2 months ago

Good point! The tickboxes are very general - to suit all the different types of software submitted to JOSS. As long as you feel that the "Statement of need" section in the paper is alright, you can check off the box. However, you could also ask the authors to have a second look at it and improve if you feel something is missing.

And thanks for starting right off - we encourage here over-commenting, so let's keep it like that :+1:

dmreagan commented 2 months ago

Review checklist for @dmreagan

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

hollowscene commented 2 months ago

Hello, just to update, I've made a bit more of a dent into my checklist. Hoping to be able to work through the rest over the week ahead

dmreagan commented 2 months ago

I also wondered about the explicit target audience as in https://github.com/openjournals/joss-reviews/issues/7120#issuecomment-2298631975. But I think it is implied that the intended users are the same users of the "most graphics packages" the authors refer to in the second paragraph. It's a ✅ for me.

dmreagan commented 2 months ago

Question about references: is it OK to have web links in the body of the paper? The authors have included direct URLs to a few of their own web resources while putting all 3rd-party links down in the References section.

jromanowska commented 2 months ago

Question about references: is it OK to have web links in the body of the paper? The authors have included direct URLs to a few of their own web resources while putting all 3rd-party links down in the References section.

For me, that seems logical. However, if you think it doesn't suit, go ahead and suggest another way of doing it. JOSS does not have strict formatting rules for the papers.

hollowscene commented 2 months ago

Just finished my review checklist.

I did encounter one issue with some tests failing due to some Windows OS discrepancies which I've raised a GitHub issue for. Regardless, automated tests are thorough and well documented so I've still ticked it off in the checklist.

Regarding references, adding 3rd-party links seems reasonable and is more quickly accessible so I've also given it a tick.

Overall, the paper is well written and the package has had significant work dedicated to it. I have no doubt this package will be incredibly useful for both casual and veteran data vizzers. Amazing work :)

zeileis commented 2 months ago

Andrew @hollowscene, thank you so much for the positive and constructive feedback along with the issues and pull requests in the GitHub project. Reto is currently on vacation but we'll incorporate all your helpful suggestions in the software and paper when he's back! Thanks & best wishes, Achim

jromanowska commented 1 month ago

@zeileis , @retostauffer - how is it going? Let us know when you're done with the modifications.

zeileis commented 1 month ago

Julia @jromanowska, thanks for the reminder and apologies for the delay. Reto will be back from his vacations this week. I already took some notes and I expect that we will have a revision and response to the helpful reviews very soon.

zeileis commented 1 month ago

Another follow-up, unfortunately just to say that we will need a bit more time because corona had other plans for us this week. But we promise that we will get back to this as soon as possible. Apologies again!

jromanowska commented 1 month ago

Another follow-up, unfortunately just to say that we will need a bit more time because corona had other plans for us this week. But we promise that we will get back to this as soon as possible. Apologies again!

Thanks for letting us know and stay safe. Get healthy while we're waiting patiently :)

retostauffer commented 1 month ago

Dear @jromanowska, @hollowscene, and @dmreagan,

First of all, we would like to sincerely thank you for handling our submission, as well as for the constructive and valuable feedback on our work. We apologize for the delayed response due to vacations and covid.

We have thoroughly addressed all comments from the review as well as the pre-review (handled by Daniel S. Katz).

We have also updated the README to acknowledge the contributions of the reviewers and editors involved in this process. Your efforts in enhancing the quality of our work are greatly appreciated.

Paper

Software

Key Changes

Additional Changes

Once again, thank you for your time and thoughtful feedback. We look forward to your further comments and suggestions.

Best regards,
@retostauffer and @zeileis

retostauffer commented 1 month ago

@editorialbot generate pdf

editorialbot commented 1 month ago

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

jromanowska commented 3 weeks ago

@hollowscene , @dmreagan - are you satisfied with the modifications and response from the authors?

retostauffer commented 3 weeks ago

Thanks, @jromanowska!

@hollowscene & @dmreagan, if you want to, you can respond to the individual issues that you raised and we have answered (https://github.com/retostauffer/python-colorspace/issues/21, https://github.com/retostauffer/python-colorspace/issues/23, https://github.com/retostauffer/python-colorspace/issues/24). Feel free to close them if you feel that they have been sufficiently addressed. Or if you respond here, we can also close them in our repository.

Thanks again for your feedback and consideration.

dmreagan commented 3 weeks ago

Everything looks good to me. I completed my checklist. Excellent work, @retostauffer and @zeileis!

jromanowska commented 2 weeks ago

Thank you all for great work and communication. I will now proceed to final checks before recommending acceptance.

jromanowska commented 2 weeks ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

jromanowska commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

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

jromanowska commented 2 weeks ago

@editorialbot check references

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

✅ OK DOIs

- 10.1038/s41467-020-19160-7 is OK
- 10.18637/jss.v096.i01 is OK
- 10.1109/tvcg.2009.113 is OK
- 10.1109/mcse.2007.55 is OK
- 10.21105/joss.03021 is OK
- 10.5281/zenodo.10957263 is OK
- 10.5281/zenodo.12514964 is OK
- 10.1038/s41586-020-2649-2 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Color Use Guidelines for Data Representation
- No DOI given, and none found for title: Color
- No DOI given, and none found for title: Fundamentals of Data Visualization
- No DOI given, and none found for title: What’s New in Matplotlib 2.0, Changes to the Defau...
- No DOI given, and none found for title: Colour for Presentation Graphics
- No DOI given, and none found for title: Colormap
- No DOI given, and none found for title: Colormaps
- No DOI given, and none found for title: palettable: Color Palettes for Python
- No DOI given, and none found for title: cmcrameri: Python Wrapper around Fabio Crameri’s P...

❌ MISSING DOIs

- 10.1016/0306-4573(91)90096-5 may be a valid DOI for title: Envisioning Information

❌ INVALID DOIs

- None
jromanowska commented 2 weeks ago

@retostauffer - please add "Austria" to the author affiliations in the paper and go through the points above under Additional Author Tasks After Review is Complete

retostauffer commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

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

retostauffer commented 2 weeks ago

To clarify: The editorialbot flagged a missing DOI (10.1016/0306-4573(91)90096-5), which actually references a review of the book, not the book itself (see further discussion).

Finally, we (and I believe I can also speak for @zeileis here) would like to express our gratitude to everyone involved for your time and efforts, from pre-screening to reviewing and managing our submission. This is our first submission to JOSS, as well as our first git-based submission, and it certainly won't be our last!

If we’ve overlooked anything, please let us know. Thank you again and all the best! @zeileis and @retostauffer

jromanowska commented 2 weeks ago

@editorialbot generate pdf

editorialbot commented 2 weeks ago

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

jromanowska commented 2 weeks ago

@editorialbot set 10.5281/zenodo.14004295 as archive

editorialbot commented 2 weeks ago

Done! archive is now 10.5281/zenodo.14004295

jromanowska commented 2 weeks ago

@editorialbot set v1.0.0 as version

editorialbot commented 2 weeks ago

Done! version is now v1.0.0

jromanowska commented 2 weeks ago

Thank you, @dmreagan, @hollowscene for great work reviewing. Thank you, @retostauffer, @zeileis for the submission and work with improving the software. :clap:

The editor-in-chief will now take over for final round of checks before publication.

jromanowska commented 2 weeks ago

@editorialbot recommend-accept

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

✅ OK DOIs

- 10.1038/s41467-020-19160-7 is OK
- 10.18637/jss.v096.i01 is OK
- 10.1109/tvcg.2009.113 is OK
- 10.1109/mcse.2007.55 is OK
- 10.21105/joss.03021 is OK
- 10.5281/zenodo.10957263 is OK
- 10.5281/zenodo.12514964 is OK
- 10.1038/s41586-020-2649-2 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Color Use Guidelines for Data Representation
- No DOI given, and none found for title: Color
- No DOI given, and none found for title: Fundamentals of Data Visualization
- No DOI given, and none found for title: What’s New in Matplotlib 2.0, Changes to the Defau...
- No DOI given, and none found for title: Colour for Presentation Graphics
- No DOI given, and none found for title: Colormap
- No DOI given, and none found for title: Colormaps
- No DOI given, and none found for title: palettable: Color Palettes for Python
- No DOI given, and none found for title: cmcrameri: Python Wrapper around Fabio Crameri’s P...

❌ MISSING DOIs

- 10.1016/0306-4573(91)90096-5 may be a valid DOI for title: Envisioning Information

❌ INVALID DOIs

- None
editorialbot commented 2 weeks ago

:wave: @openjournals/csism-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/6062, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

zeileis commented 2 weeks ago

Quick feedback regarding the DOI suggested as MISSING above. This was already addressed in the pre-review (see https://github.com/openjournals/joss-reviews/issues/7053#issuecomment-2258039140). The DOI should not be included as it is for a review of the book and not the book itself.

I also looked at the PDF and XML files and just noticed a minor aspect: https://github.com/openjournals/joss-papers/pull/6062#pullrequestreview-2401480863

danielskatz commented 2 weeks ago

👋 @zeileis - as the track editor, I'll proofread this next and let you know about anything else you need to do. I've also asked a couple of other people about the incorrect copyright date, and we won't publish until that's fixed/understood.