search

openjournals / joss-reviews

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

[REVIEW]: UnfoldSim.jl: A toolbox for simulating continuous event-based time series data for EEG and beyond #6641

Open editorialbot opened 4 months ago

editorialbot commented 4 months ago

Submitting author: !--author-handle-->@jschepers<!--end-author-handle-- (Judith Schepers) Repository: https://github.com/unfoldtoolbox/UnfoldSim.jl Branch with paper.md (empty if default branch): joss-paper Version: v.0.3.1 Editor: !--editor-->@mooniean<!--end-editor-- Reviewers: @cbrnr, @aurorarossi, @roualdes Archive: Pending

Status

status

Status badge code:

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

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

@jonbmartin & @cbrnr & @aurorarossi & @roualdes, 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 @mooniean 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 @cbrnr

📝 Checklist for @roualdes

📝 Checklist for @aurorarossi

editorialbot commented 4 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 4 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.7717/peerj.7838 is OK
- 10.1167/jov.21.1.3 is OK
- 10.1088/1741-2552/aca8ce is OK
- 10.1137/141000671 is OK
- 10.18637/jss.v107.i04 is OK
- 10.5281/zenodo.2647458 is OK
- 10.18637/jss.v098.i16 is OK
- 10.5281/zenodo.8344531 is OK
- 10.21105/joss.03349 is OK
- 10.5281/zenodo.10268806 is OK
- 10.5281/zenodo.10669002 is OK
- 10.5281/zenodo.10214175 is OK
- 10.5281/zenodo.10235220 is OK
- 10.3389/fnins.2013.00267 is OK
- 10.21105/joss.05848 is OK
- 10.18502/fbt.v10i3.13165 is OK
- 10.1016/j.jneumeth.2020.109017 is OK
- 10.3390/s21113632 is OK
- 10.1016/j.jneumeth.2019.108377 is OK
- 10.1155/2011/879716 is OK
- 10.1016/j.jneumeth.2020.109017 is OK
- 10.1109/TNSRE.2018.2873061 is OK

MISSING DOIs

- No DOI given, and none found for title: Documenter: A documentation generator for Julia

INVALID DOIs

- None
editorialbot commented 4 months ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.05 s (1259.7 files/s, 184107.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              4              0              0           4462
Julia                           37            580            631           1921
TeX                              1             26              6            356
Markdown                         4             90              0            263
YAML                             7              7             14            177
TOML                             6              5              0             85
-------------------------------------------------------------------------------
SUM:                            59            708            651           7264
-------------------------------------------------------------------------------

Commit count by author:

   138  jschepers
    80  behinger (s-ccs 001)
    64  Benedikt Ehinger
    20  allcontributors[bot]
    14  Judith Schepers
     7  behinger
     4  llips
     2  CompatHelper Julia
     2  Luis Lips
     2  Manpa Barman
     2  maanikmarathe
     1  Maanik Marathe
     1  René Skukies
     1  Vladimir Mikheev
editorialbot commented 4 months ago

Paper file info:

📄 Wordcount for paper.md is 1984

✅ The paper includes a Statement of need section

editorialbot commented 4 months ago

License info:

✅ License found: MIT License (Valid open source OSI approved license)

editorialbot commented 4 months ago

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

mooniean commented 4 months ago

Hi @jschepers and reviewers @cbrnr @roualdes @aurorarossi and @jonbmartin - this is the review thread for the submission. All of our communications will happen here from now on. The title was updated on the pre-review but I'm not sure why it wasn't updated here - I'll try to sort it!

Meanwhile, please check the post at the top of the issue for instructions on how to generate your own review checklist. As you go over the submission, please check any items that you feel have been satisfied. There are also 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 directly in the software repository. If you do so, please mention this thread so that a link is created (and I can keep an eye on what is happening). Please also feel free to comment and ask questions in this thread. It is often easier to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

Please feel free to ping me (@mooniean) if you have any questions or concerns. Thanks!

cbrnr commented 4 months ago

Review checklist for @cbrnr

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

roualdes commented 4 months ago

Review checklist for @roualdes

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

aurorarossi commented 4 months ago

Review checklist for @aurorarossi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

aurorarossi commented 4 months ago

@jschepers

cbrnr commented 4 months ago

I'm struggling a bit with the following point:

The first part is pretty clear, the submitting authors has definitely made major contributions to the software. However, I'm not sure that this is also the case for the second author @llips, who has only one trivial commit (see https://github.com/unfoldtoolbox/UnfoldSim.jl/graphs/contributors). Since there is no author contributions section, I would argue that this author should probably not be on the authors list based on the information I have. But I'm happy to hear any clarification of course!

cbrnr commented 4 months ago

Regarding the paper, I agree with @aurorarossi that it is currently too long. I wouldn't reduce the Simulation chapter though, because although it can be found in the documentation, this is the gist of the entire package. I think a few sentences/paragraphs can be removed without too much negative effects throughout the entire manuscript. I would especially try to shorten the validation part, which, although very interesting, is not the focus of this package, and at least for me it raises several questions that are better discussed elsewhere. I agree that the package references can be completely removed (and if you want to credit these packages, maybe just one sentence pointing to some part of the docs would be sufficient). Also, the "Related tools" section could be considerably shortened.

I also have a couple of suggestions that I think would improve the paper:

Once these issues are resolved, I think the paper is good to go and I will focus on the software.

mooniean commented 4 months ago

Hi @jschepers and reviewers @cbrnr @roualdes @aurorarossi and @jonbmartin ! Just checking in how things are going!

cbrnr commented 4 months ago

I'd like to wait for @jschepers to reply to my comments before I continue with my review.

jschepers commented 4 months ago

Hello, I thought I'd wait until I got all the reviews and then start addressing them. But then I will already start working on them now. Thank you @aurorarossi and @cbrnr for your detailed comments and questions.

roualdes commented 4 months ago

I've been actively following along, but haven't found the time yet to get my review going. This is high on my list for early next week.

jonbmartin commented 4 months ago

Hello Beatriz,

I think there may have been a miscommunication - I initially said that I was unable to review. I am currently working on some other reviews for JOSS and other journals. I will be happy to review in the future!

Very best, Jonathan Martin

On Wed, May 8, 2024 at 5:32 AM Beatriz Costa Gomes @.***> wrote:

Hi @jschepers https://github.com/jschepers and reviewers @cbrnr https://github.com/cbrnr @roualdes https://github.com/roualdes @aurorarossi https://github.com/aurorarossi and @jonbmartin https://github.com/jonbmartin ! Just checking in how things are going!

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/6641#issuecomment-2100269794, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKVCUT7JLTRT7MPMPZJSSZLZBH5LZAVCNFSM6AAAAABGLVEY2SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBQGI3DSNZZGQ . You are receiving this because you were mentioned.Message ID: @.***>

mooniean commented 4 months ago

@jonbmartin Apologies! I'll try to sort this out. Thank you for your time!

mooniean commented 4 months ago

@editorialbot remove @jonbmartin from reviewers

editorialbot commented 4 months ago

@jonbmartin removed from the reviewers list!

roualdes commented 4 months ago

I filed a new issue, https://github.com/unfoldtoolbox/UnfoldSim.jl/issues/95, with all my doc comments and suggestions. I'll wait for updates on the paper before I read that over.

jschepers commented 4 months ago

I filed a new issue, unfoldtoolbox/UnfoldSim.jl#95, with all my doc comments and suggestions. I'll wait for updates on the paper before I read that over.

Thanks a lot for your detailed comments on the documentation. That's very helpful.

mooniean commented 3 months ago

Hi @jschepers and reviewers @cbrnr @roualdes @aurorarossi! Checking in that everything is going smoothly!

jschepers commented 3 months ago

@mooniean Thank you for checking. Everyone gave very useful suggestions. Thanks again. Unfortunately, I did not have time to address them yet. I will allocate time for this soon.

danielskatz commented 2 months ago

👋 @mooniean - As track editor, I try to check in on reviews where nothing has happened in a couple of weeks. How is this going? And can I do anything to help it move forward?

mooniean commented 2 months ago

@danielskatz thanks for checking in! Author has asked for some time to address the comments from reviewers :)

mooniean commented 1 month ago

Hi @jschepers ! Checking in on the review to see how you're doing with the changes!

jschepers commented 1 month ago

Dear all, First of all, we thank you for being patient with our late reply. Thanks a lot again for all your helpful feedback on the paper and the package. Please find the changes we made and the additional clarifications in separate posts below. Benedikt & Judith

jschepers commented 1 month ago

Dear @aurorarossi, thanks a lot for your suggestion about adding a CONTRIBUTING.md file and the helpful links to examples. We added the following CONTRIBUTING.md file: https://github.com/unfoldtoolbox/UnfoldSim.jl/blob/main/CONTRIBUTING.md Please let us know whether this matches your ideas about the contribution file.

cbrnr commented 1 month ago

@editorialbot generate pdf

jschepers commented 1 month ago

Dear @cbrnr, thank you for your detailed suggestions about the paper. Please find our changes to the paper and additional explanations in the following PR: https://github.com/unfoldtoolbox/UnfoldSim.jl/pull/103 Please let us know if your questions have not yet been sufficiently answered or if additional changes should be made.

jschepers commented 1 month ago

@cbrnr Thanks a lot for pointing this out. Luis Lips' contribution to the project is indeed not clear from the UnfoldSim repository alone. However, in his master thesis (that @behinger and I supervised), he worked on "Statistically evaluating mixed-effects models for EEG analysis using large-scale simulations" and one part of his thesis was about implementing a framework to simulate EEG data. A revised version of this became the foundation for the UnfoldSim package. His master thesis code can be found in this repository.

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:

jschepers commented 1 month ago

Dear @roualdes, thanks a lot for reviewing the package documentation. As developer of the package, one often becomes blind to many aspects, and your perspective has been very helpful to restructure the documentation and make it more understandable. Please find our revision of the documentation in this PR: https://github.com/unfoldtoolbox/UnfoldSim.jl/pull/101 Please let us know if this addresses your suggestions regarding the package documentation or if anything is still missing

jschepers commented 1 month ago

@aurorarossi and @cbrnr Thank you for your comments about the length of the paper. We can definitely see that our paper draft is rather long for a JOSS paper. However, we would like to keep both the simulation example and the references to other packages:

We think that the simulation example gives a good overview of the functionalities of the paper and offers readers a good starting point (even before having a look at the documentation). Additionally, it would be helpful to have a non-modifiable record of the tool's use cases for academic purposes (especially to use it in a cumulative dissertation). One alternative option would be to provide an additional arXiv version, but that seems cumbersome and confusing.

Regarding the package references, we would like to follow the idea of "giving credit where credit is due" and want to highlight the efforts that researchers put into research software development. This potentially helps establish Research Software Engineering as a viable academic path.

Given that the statement about the word count is not a "must" formulation we hope that our explanations convince you to keep the manuscript also with a quite extended length.

cbrnr commented 1 month ago

Thanks for addressing my points, as I've already mentioned in https://github.com/unfoldtoolbox/UnfoldSim.jl/pull/103, I'm very happy with the changes! Regarding the paper length, I totally understand that these things are important, but given the journal constraints I'd still say there's room for reducing the length a bit. However, I'll leave this for the journal editors @mooniean @danielskatz to decide.

I'll be reviewing the software package next, but please note that I am currently busy with other things and will be on vacation soon, so it might take me until mid-September to finalize my review. I hope that's OK.

cbrnr 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:

danielskatz commented 1 month ago

@editorialbot check repository

editorialbot commented 1 month ago

Software report:

github.com/AlDanial/cloc v 1.90  T=0.06 s (1039.7 files/s, 185524.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                              4              0              0           4038
Julia                           39           1084           2716           2180
TeX                              1             26              6            356
Markdown                         5             89              0            281
YAML                             7              7             14            177
TOML                             6              5              0             84
-------------------------------------------------------------------------------
SUM:                            62           1211           2736           7116
-------------------------------------------------------------------------------

Commit count by author:

   138  jschepers
    80  behinger (s-ccs 001)
    64  Benedikt Ehinger
    20  allcontributors[bot]
    15  Judith Schepers
     7  behinger
     4  llips
     2  CompatHelper Julia
     2  Luis Lips
     2  Manpa Barman
     2  maanikmarathe
     1  Maanik Marathe
     1  René Skukies
     1  Vladimir Mikheev
editorialbot commented 1 month ago

Paper file info:

📄 Wordcount for paper.md is 2029

✅ The paper includes a Statement of need section

editorialbot commented 1 month ago

License info:

✅ License found: MIT License (Valid open source OSI approved license)

danielskatz commented 1 month ago

👋 sorry to have missed the notification above. Given that this paper is slightly over 2000 words, and the JOSS standard is up to 1000, I think some of it does need to be cut. My two suggestions to cut this and the overall number of pages to make it more like other JOSS papers are

  1. Either change the functionality section to be a single paragraph, with the details in the documentation, or move the example to the documentation. I think the first would be better, as I like having an example in the paper.
  2. Change the package references section to be a paragraph with items separated by commas, rather than having each item be its own line/paragraph.
danielskatz commented 3 weeks ago

@jschepers - What is your plan for reducing the paper? And have you started on it?

jschepers commented 3 weeks ago

@danielskatz I'm sorry for the late reply. I am currently on vacation/conferences and will be back in the office at the beginning of September. Thank you for your suggestions regarding the paper. I will implement the changes to the package references section and will discuss the shortening options with Benedikt. We will also go through the whole paper again and decide which information we can leave out. Since the word limit is not a "must" formulation we hoped that we could keep the paper in its current structure because we think that the extra information is valuable to the readers. Moreover, it would be valuable to me to have the paper in its long form for a cumulative dissertation. As discussed before, we have to check back with the University committee if we need a "full" ArXiv version, which would be a bit redundant. But of course we also understand the goal of JOSS to keep papers within the word limit. If we do not find enough other content to shorten I currently have a tendency to changing the functionality section to a single paragraph because I also would like to keep the example.