search

openjournals / joss-reviews

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

[REVIEW]: GasChromatographySimulator.jl #4565

Closed editorialbot closed 2 years ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@JanLeppert<!--end-author-handle-- (Jan Leppert) Repository: https://github.com/JanLeppert/GasChromatographySimulator.jl Branch with paper.md (empty if default branch): Version: 0.3.10 Editor: !--editor-->@rkurchin<!--end-editor-- Reviewers: @ma-sadeghi, @bonh Archive: 10.5281/zenodo.6992295

Status

status

Status badge code:

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

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

@ma-sadeghi & @bonh, 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 @rkurchin 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 @bonh

📝 Checklist for @ma-sadeghi

editorialbot commented 2 years 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 years ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.11 s (420.3 files/s, 198535.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           15           1978            847          10198
SVG                             12              0              0           7242
Markdown                         8            220              0            508
YAML                             4              3              6            103
TeX                              1              8              0             86
TOML                             4              4              0             49
JSON                             1              0              0              2
-------------------------------------------------------------------------------
SUM:                            45           2213            853          18188
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Wordcount for paper.md is 584

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

OK DOIs

- 10.1016/j.chroma.2019.460645 is OK
- 10.1016/j.chroma.2020.460985 is OK
- 10.1002/jssc.201701343 is OK
- 10.1137/141000671 is OK
- 10.5334/jors.151 is OK
- 10.1016/j.chroma.2021.462300 is OK
- 10.1016/j.chroma.2019.460696 is OK
- 10.5281/ZENODO.4792401 is OK
- 10.1016/j.chroma.2017.02.047 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

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

bonh commented 2 years ago

Review checklist for @bonh

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

rkurchin commented 2 years ago

Reminder to @ma-sadeghi and @bonh to get your reviews started!

ma-sadeghi commented 2 years ago

@rkurchin I'm a bit busy until this Wednesday, I'll get started in a few days, and will try to make up for the delay. I'm sorry for that.

ma-sadeghi commented 2 years ago

Review checklist for @ma-sadeghi

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

bonh commented 2 years ago

Hi @JanLeppert! So far the packages looks awesome, thanks. I am trying to look into this reference http://10.1016/j.chroma.2020.460985 but I cannot download it. Can you provide me a preprint?

bonh commented 2 years ago

Especially the documentation is outstanding.

bonh commented 2 years ago

@JanLeppert, I cannot find anything about "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".

JanLeppert commented 2 years ago

Hi @JanLeppert! So far the packages looks awesome, thanks. I am trying to look into this reference http://10.1016/j.chroma.2020.460985 but I cannot download it. Can you provide me a preprint?

Thank you for your comments. Below you can find a copy of the reference you ask for: Leppert.2020b-Simulation of spatial thermal gradient gas chromatography-Annotation.pdf

rkurchin commented 2 years ago

👋 hi @ma-sadeghi, just checking in whether you've had a chance to start your review?

bonh commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

bonh commented 2 years ago

Thanks, @JanLeppert, for providing the (great!) paper. @rkurchin, I think that this submission together with this published paper, see pdf provided by @JanLeppert above, might be a Co-publication of science, methods, and software. This is somewhat indicated by @JanLeppert in both the JOSS paper and the documentation. I still think that JOSS should consider this submission because in my opinion "the implementation of the software itself reflects a substantial scientific effort" especially due to the extensive and instructive documentation. I think this is an editorial decision, right? So before I continue my review I want to make sure that it is worth the effort.

@JanLeppert, if @rkurchin agrees to move forward with your submission (which I hope!) you should "indicate whether related publications (published, in review, or nearing submission) exist as part of submitting to JOSS", see link above. In my opinion, the paper and this submission are both valuable but perhaps you would like to mention the paper more prominently in your JOSS paper (already in the summary?).

Do produce the results of that paper did you use a predecessor or even an early version of GasChromatographySimulator.jl? If you it might make sense to use that paper to provide validating results of your software. Otherwise you might want to add a plot to the JOSS paper indicating what kind of results including their accuracy one can expect from your software.

bonh commented 2 years ago

@JanLeppert, let's not discuss the points concerning the JOSS paper I raised above here but directly over at https://github.com/JanLeppert/GasChromatographySimulator.jl.

rkurchin commented 2 years ago

Yes, @bonh, thanks for checking – I agree that this should be fine to publish in JOSS, since the paper is a detailing of the mathematical model but makes very little reference to the software, while this submission is focused on the software and its capabilities. I definitely agree that showing how to reproduce results shown in the theory paper would be excellent content for the project docs, and also that it would be good to reference that paper in your manuscript here, as an example usage (if this package was indeed used for the analysis therein), or at least as related/foundational work!

ma-sadeghi commented 2 years ago

@rkurchin Yes, I've started the review process, and I've already opened up a few issues on the package issue tracker.

@JanLeppert, I really like your package, and I think it's a worthy contribution. Please address the issues I opened up and we'll take it from there.

JanLeppert commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

JanLeppert commented 2 years ago

@bonh , @ma-sadeghi Thank you for your comments. I made some changes to the package (comments from @ma-sadeghi), added an examples section to the documentation and reworked the draft of the paper.

ma-sadeghi commented 2 years ago

@JanLeppert Thank you for your detailed responses. I opened up two more issues (60, 61), they're sort of optional, but I still recommend addressing them.

@rkurchin I'm happy with the responses/actions @JanLeppert has taken (you can find the details in the issue tracker of his package). I've opened up two more issues though. On my end, once those are addressed, I consider this submission to be complete and ready for publication. Please let me know if you need anything else from me.

@JanLeppert Congrats!

JanLeppert commented 2 years ago

@ma-sadeghi Thank you for your comments. They really helped to improve the package.

I addressed your two additional issues (60, 61)

JanLeppert commented 2 years ago

@bonh Thank you also for your helpful comments. I think, I addressed the two last issues (62, 50). The example notebooks should be functional now and I added the text about contribution also to the documentation.

JanLeppert commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

bonh commented 2 years ago

Thanks, @JanLeppert . There seems to be something wrong with the reference starting line 100.

JanLeppert commented 2 years ago

@bonh Yes, the name of the 4th author is in greek letters and only the capital greek letters are printed in the pdf. The name is written this way in the exported bibtex-file from the Zenodo-doi. I never had this case before.

bonh commented 2 years ago

Perhaps @rkurchin knows a trick?

bonh commented 2 years ago

Sorry, I just added a last issue. I think that's it.

rkurchin commented 2 years ago

I haven't seen that issue before, but my first instinct would be to tweak the .bib entry to use the TeX way of specifying Greek characters (i.e. \Pi \alpha \nu ...) and see if that fixes it. If that doesn't work, we can tag in someone more familiar with the back end of the typesetting.

JanLeppert commented 2 years ago

@rkurchin \alpha is ignored by the compiler. Using $\alpha$ prints as \alpha in the document. For the greek characters, like γ, a warning is given by the compiler Warning: Missing character: There is no γ (U+03B3) (U+03B3) in font [lmsans10-regular]:+tlig;! The regular font of the paper has no characters for the small greek letters.

Would it be appropriate to change the name to a version using latin characters?

bonh commented 2 years ago

I am done with my review. After the reference issue is resolved I recommend publication. Thanks, @JanLeppert !

rkurchin commented 2 years ago

Thanks, reviewers!

@JanLeppert, I've reached out to the JOSS leadership team about this typesetting issue and will let you know how to proceed as soon as I hear back! Thanks for your patience in the meantime.

arfon commented 2 years ago

@bonh Yes, the name of the 4th author is in greek letters and only the capital greek letters are printed in the pdf. The name is written this way in the exported bibtex-file from the Zenodo-doi. I never had this case before.

@tarleb – do you have any suggestions ons how to handle this?

tarleb commented 2 years ago

I've added support for Greek letters. It's not perfect, the kerning between uppercase and lowercase chars seems a bit off, but I'm not sure it would be worth it to spend more time on this.

tarleb commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

rkurchin commented 2 years ago

Thanks everyone! Authors, I'll do an editorial pass over the manuscript and send any comments shortly. In the meantime, the next steps for you are:

  1. Merge any and all changes from this review into your main branch and issue a new version tag. (If you want to merge in the paper, you may, but it is not required that the actual manuscript live into the repo in perpetuity since JOSS will host it and you can simply add a badge link or whatever you like. But the actual changes to software and docs do need to be merged!)
  2. Create a DOI for the contents of the repo at the same commit corresponding to that version tag, e.g. using figshare or Zenodo. Please make sure that the metadata (version number, title, author list, etc.) match those of your manuscript.
  3. Post a comment here with the version number and DOI.
rkurchin commented 2 years ago

Some editorial comments:

JanLeppert commented 2 years ago
  • 30, 32: should $t(x)$ actually be (x,t) or something like that? It seems like you're just using it to refer to the coordinate system, but I'm not entirely certain...

@rkurchin With $t(x)$ I just wanted to express, that the solution of the first ODE gives the time a substance needs to travel a certain distance. In most cases the movement of an object is described as the distance traveled in a certain time x(t).

I made the other changes you suggested.

JanLeppert commented 2 years ago

@rkurchin The repo is registered as version 0.3.10 and the DOI 10.5281/zenodo.6992295 was created.

rkurchin commented 2 years ago

@editorialbot check references

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

OK DOIs

- 10.1016/j.chroma.2019.460645 is OK
- 10.1016/j.chroma.2020.460985 is OK
- 10.1002/jssc.201701343 is OK
- 10.1137/141000671 is OK
- 10.5334/jors.151 is OK
- 10.1016/j.chroma.2021.462300 is OK
- 10.1016/j.chroma.2019.460696 is OK
- 10.5281/zenodo.6234110 is OK
- 10.1016/j.chroma.2017.02.047 is OK
- 10.1016/j.chroma.2019.460737 is OK

MISSING DOIs

- None

INVALID DOIs

- None
rkurchin commented 2 years ago

@editorialbot set 0.3.10 as version

editorialbot commented 2 years ago

Done! version is now 0.3.10

rkurchin commented 2 years ago

@editorialbot set 10.5281/zenodo.6992295 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.6992295