openjournals / joss-reviews

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

[REVIEW]: libsoni: A Python Toolbox for Sonifying Music Annotations and Feature Representations #6524

Closed editorialbot closed 3 months ago

editorialbot commented 6 months ago

Submitting author: !--author-handle-->@yiitozer<!--end-author-handle-- (Yigitcan Özer) Repository: https://github.com/groupmm/libsoni Branch with paper.md (empty if default branch): paper Version: v1.0.2 Editor: !--editor-->@osorensen<!--end-editor-- Reviewers: @expectopatronum, @ren-zeng Archive: 10.5281/zenodo.11085871

Status

status

Status badge code:

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

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

@expectopatronum & @ren-zeng, 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 @osorensen 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 @ren-zeng

📝 Checklist for @expectopatronum

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

Software report:

github.com/AlDanial/cloc v 1.90  T=0.09 s (1135.7 files/s, 378623.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
CSV                             28              0              0          18880
HTML                            21            891             60           4241
SVG                              1              0              0           2671
Python                          12            543            737            959
JavaScript                      12            131            213            871
CSS                              4            189             35            776
Jupyter Notebook                 5              0           1363            498
Markdown                         2             28              0            232
TeX                              1             14              0            133
YAML                             2              3              4             45
reStructuredText                11             20             46             33
DOS Batch                        1              8              1             26
make                             1              4              7              9
-------------------------------------------------------------------------------
SUM:                           101           1831           2466          29374
-------------------------------------------------------------------------------

Commit count by author:

   145  leob
    78  yiitozer
    69  Leo Brütting
    22  Yigitcan Oezer
     4  Yigitcan Özer
     2  Øystein Sørensen
editorialbot commented 6 months ago

Paper file info:

📄 Wordcount for paper.md is 2366

✅ The paper includes a Statement of need section

editorialbot commented 6 months ago

License info:

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

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

OK DOIs

- 10.5281/zenodo.7512227 is OK
- 10.21105/joss.03434 is OK
- 10.21105/joss.03326 is OK
- 10.1109/MSP.2018.2869928 is OK
- 10.1109/MSP.2018.2875349 is OK
- 10.1145/2964284.2973795 is OK
- 10.1007/978-3-319-21945-5 is OK
- 10.25080/Majora-7b98e3ed-003 is OK
- 10.5281/zenodo.1415016 is OK
- 10.5281/zenodo.1416032 is OK
- 10.5281/zenodo.849741 is OK
- 10.1145/1631272.1631459 is OK
- 10.5281/zenodo.1417145 is OK

MISSING DOIs

- No DOI given, and none found for title: Tempogram Toolbox: MATLAB Tempo and Pulse Analysis...
- No DOI given, and none found for title: Sonification Report: Status of the Field and Resea...

INVALID DOIs

- None
editorialbot commented 6 months ago

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

osorensen commented 5 months ago

👋 @expectopatronum, @ren-zeng, can you please update us on how it's going with your reviews?

ren-zeng commented 5 months ago

Review checklist for @ren-zeng

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ren-zeng commented 5 months ago

👋 @expectopatronum, @ren-zeng, can you please update us on how it's going with your reviews?

Hi @osorensen, thanks for the reminder. I had some deadlines earlier and now I can mainly focus on this review. Sorry for the delay.

expectopatronum commented 5 months ago

👋 @expectopatronum, @ren-zeng, can you please update us on how it's going with your reviews?

Hi! Thanks for the reminder! I also had a deadline (submitted on Saturday) and was planning to work on the review this week!

expectopatronum commented 5 months ago

Review checklist for @expectopatronum

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ren-zeng commented 5 months ago

@yiitozer

yiitozer commented 5 months ago

Dear @ren-zeng, thank you very much for your constructive suggestions.

  1. Performance: We will include a comparative analysis with librosa and libfmp in the Jupyter Notebooks.

  2. Functionality documentation:

    • We indeed provided minimal examples per core function in the Jupyter notebooks. Shall we also integrate them to the API?
    • The explicit explanation of the dimension is a good point, we will add the shapes of the resulting arrays.
  3. Automated tests:

    • Since the toolbox is designed to sonify input annotations, we chose not to use randomly generated inputs to have a controlled scenario. Could you please elaborate on this suggestion?
    • We will also add tests for the spectrogram case, where the input dimensions are more complex.
    • We will add negative examples, thank you for your suggestion.
yiitozer commented 5 months ago

Dear @ren-zeng,

Performance: We have chosen to remove this statement, as the basic functions provided by libfmp and librosa show similar performances as those by libsoni: https://github.com/groupmm/libsoni/commit/859ea29843cd7a886f098e763baa8ef5e60c9458

Functionality documentation: We updated the API documentation indicating the explicit shapes of input and output arrays: https://groupmm.github.io/libsoni/build/html/index.html However, since we provided minimal examples of the core functions in the notebooks, adding those to the API documentation would result in redundant duplication of those examples.

Automated tests: We changed the tests to the comprehensive testing framework unittest and added further test cases, including negative examples. Since the toolbox is designed to sonify input annotations, we chose not to use randomly generated inputs to have a controlled scenario. Could you please elaborate on this suggestion?

ren-zeng commented 5 months ago

Dear @yiitozer,

Thank you for your update on the package and the paper. On my side, everything looks good now. Regarding my comment on the randomly generated inputs, I was mainly concerned, as a potential user, about what kinds of inputs are valid. But since you already added the required array shapes, it is not necessary anymore.

Hi @osorensen, I have checked all the boxes and my review is finished.

expectopatronum commented 5 months ago

I am deeply sorry for taking so long! I (once again) overestimated how much time I would have with a toddler at home 🙃

After solving a few issues directly by opening a Github issue, there are only a few things left:

Besides that, the package is great and I would have loved to have something like this when I started my PhD! I think it will be very useful for research in MIR and I think it will be easy to get started with it thanks to the example notebooks (which is not surprising coming from this group :)).

Thanks for your patience and best regards Verena

osorensen commented 5 months ago

Thanks a lot for completing your review @ren-zeng!

osorensen commented 5 months ago

Thanks a lot for your comments @expectopatronum. @yiitozer, please let us know here when the additional issues pointed out by @expectopatronum have been addressed.

yiitozer commented 5 months ago

Dear @expectopatronum,

* Documentation: It took me a while to find the [link to the documentation](https://groupmm.github.io/libsoni/build/html/index.html) (in the sidebar of the github repo), maybe it could also be linked somewhere directly in the README?

* For me it is not entirely clear whether by "Documentation" (in the review check list) the linked documentation is meant or the README + the linked documentation. In the first case I cannot check some of the boxes, because the installation instructions, example usage, ... is not described in what I think is the "documentation". Maybe it would be sufficient to mention in the starting page of the documentation that more details (installation instructions, ...) can be found in the Github repository. I'm thinking of the case where someone finds the package github.io page and does not know about the repository.

Thank you very much for your suggestions, we added a link to the documentation in the README file and a link to the GitHub repository in the API documentation, as well as renamed licence to license :) : https://github.com/groupmm/libsoni/pull/13

Besides that, the package is great and I would have loved to have something like this when I started my PhD! I think it will be very useful for research in MIR and I think it will be easy to get started with it thanks to the example notebooks (which is not surprising coming from this group :)).

Thanks for your patience and best regards Verena

Thank you very much for your constructive and motivating comment!! :) We're very happy to get such feedback!

Best, Yigit

yiitozer commented 5 months ago

Dear @osorensen, we're done with our changes based on the reviews!

osorensen commented 5 months ago

Thanks for responding so quickly, @yiitozer! @expectopatronum, could you please go through the changes, and update your review checklist if your points have been addressed?

expectopatronum commented 5 months ago

Dear @osorensen, everything looks great now, and all points are checked.

osorensen commented 5 months ago

@editorialbot generate pdf

osorensen commented 5 months ago

@editorialbot check references

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

OK DOIs

- 10.5281/zenodo.7512227 is OK
- 10.21105/joss.03434 is OK
- 10.21105/joss.03326 is OK
- 10.1109/MSP.2018.2869928 is OK
- 10.1109/MSP.2018.2875349 is OK
- 10.1145/2964284.2973795 is OK
- 10.1007/978-3-319-21945-5 is OK
- 10.25080/Majora-7b98e3ed-003 is OK
- 10.5281/zenodo.1415016 is OK
- 10.5281/zenodo.1416032 is OK
- 10.5281/zenodo.849741 is OK
- 10.1145/1631272.1631459 is OK
- 10.5281/zenodo.1417145 is OK

MISSING DOIs

- No DOI given, and none found for title: Tempogram Toolbox: MATLAB Tempo and Pulse Analysis...
- No DOI given, and none found for title: Sonification Report: Status of the Field and Resea...

INVALID DOIs

- None
editorialbot commented 5 months ago

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

osorensen commented 5 months ago

@yiitozer, I have now read through the final manuscript and have nothing to add or correct.

At this point could you:

I can then move forward with recommending acceptance of the submission.

yiitozer commented 5 months ago

Dear @osorensen, thank you very much for the detailed instructions.

osorensen commented 5 months ago

@editorialbot set v1.0.2 as version

editorialbot commented 5 months ago

Done! version is now v1.0.2

osorensen commented 5 months ago

@editorialbot set 10.5281/zenodo.11085871 as archive

editorialbot commented 5 months ago

Done! archive is now 10.5281/zenodo.11085871

osorensen commented 5 months ago

@editorialbot recommend-accept

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

OK DOIs

- 10.5281/zenodo.7512227 is OK
- 10.21105/joss.03434 is OK
- 10.21105/joss.03326 is OK
- 10.1109/MSP.2018.2869928 is OK
- 10.1109/MSP.2018.2875349 is OK
- 10.1145/2964284.2973795 is OK
- 10.1007/978-3-319-21945-5 is OK
- 10.25080/Majora-7b98e3ed-003 is OK
- 10.5281/zenodo.1415016 is OK
- 10.5281/zenodo.1416032 is OK
- 10.5281/zenodo.849741 is OK
- 10.1145/1631272.1631459 is OK
- 10.5281/zenodo.1417145 is OK

MISSING DOIs

- No DOI given, and none found for title: Tempogram Toolbox: MATLAB Tempo and Pulse Analysis...
- No DOI given, and none found for title: Sonification Report: Status of the Field and Resea...

INVALID DOIs

- None
editorialbot commented 5 months ago

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

yiitozer commented 5 months ago

Dear @osorensen, is the paper than acceptable with the command "editorialbot accept"? :)

osorensen commented 5 months ago

No, you'll have to wait for the editor-in-chief to make the final decision :)

yiitozer commented 5 months ago

Alright, thank you for your quick response! @osorensen

oliviaguest commented 5 months ago

@editorialbot recommend-accept

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

OK DOIs

- 10.5281/zenodo.7512227 is OK
- 10.21105/joss.03434 is OK
- 10.21105/joss.03326 is OK
- 10.1109/MSP.2018.2869928 is OK
- 10.1109/MSP.2018.2875349 is OK
- 10.1145/2964284.2973795 is OK
- 10.1007/978-3-319-21945-5 is OK
- 10.25080/Majora-7b98e3ed-003 is OK
- 10.5281/zenodo.1415016 is OK
- 10.5281/zenodo.1416032 is OK
- 10.5281/zenodo.849741 is OK
- 10.1145/1631272.1631459 is OK
- 10.5281/zenodo.1417145 is OK

MISSING DOIs

- No DOI given, and none found for title: Tempogram Toolbox: MATLAB Tempo and Pulse Analysis...
- No DOI given, and none found for title: Sonification Report: Status of the Field and Resea...

INVALID DOIs

- None
editorialbot commented 5 months ago

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

oliviaguest commented 5 months ago

@yiitozer please review the PR (https://github.com/groupmm/libsoni/pull/14) and then recompile the paper and let me know if this is a version you are happy to publish. ☺️

yiitozer commented 4 months ago

Dear @oliviaguest, thank you very much, I have merged the pull request. I would be more than happy to publish this version :smiley:

oliviaguest commented 4 months ago

@editorialbot recommend-accept

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

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

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

OK DOIs

- 10.5281/zenodo.7512227 is OK
- 10.21105/joss.03434 is OK
- 10.21105/joss.03326 is OK
- 10.1109/MSP.2018.2869928 is OK
- 10.1109/MSP.2018.2875349 is OK
- 10.1145/2964284.2973795 is OK
- 10.1007/978-3-319-21945-5 is OK
- 10.25080/Majora-7b98e3ed-003 is OK
- 10.5281/zenodo.1415016 is OK
- 10.5281/zenodo.1416032 is OK
- 10.5281/zenodo.849741 is OK
- 10.1145/1631272.1631459 is OK
- 10.5281/zenodo.1417145 is OK

MISSING DOIs

- No DOI given, and none found for title: Tempogram Toolbox: MATLAB Tempo and Pulse Analysis...
- No DOI given, and none found for title: Sonification Report: Status of the Field and Resea...

INVALID DOIs

- None
oliviaguest commented 4 months ago

@editorialbot generate pdf