search

openjournals / joss-reviews

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

[REVIEW]: libcdict: fast dictionaries in C #4756

Closed editorialbot closed 10 months ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@robizzard<!--end-author-handle-- (Robert G Izzard) Repository: https://gitlab.com/robizzard/libcdict Branch with paper.md (empty if default branch): Version: 1.41 Editor: !--editor-->@ivastar<!--end-editor-- Reviewers: @langmm, @pgrete Archive: 10.5281/zenodo.10287855

Status

status

Status badge code:

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

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

@langmm & @pgrete, 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 @ivastar 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 @langmm

πŸ“ Checklist for @pgrete

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.18 s (681.3 files/s, 234602.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C/C++ Header                    29           1484           1314          13558
C                               78           1708           2231          12251
Fortran 90                       1            790             24           6091
Markdown                         2            343              0           1020
HTML                             1             28              0            957
Meson                            1             44            108            320
TeX                              1             16              0            168
Python                           2             74            245            119
Perl                             1              6              5             72
Bourne Shell                     9             11             10             46
-------------------------------------------------------------------------------
SUM:                           125           4504           3937          34602
-------------------------------------------------------------------------------

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

Wordcount for paper.md is 998

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:

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

OK DOIs

- 10.1111/j.1365-2966.2004.07446.x is OK
- 10.1051/0004-6361:20066129 is OK
- 10.1051/0004-6361/200912827 is OK
- 10.1093/mnras/stx2355 is OK
- 10.1002/spe.2984 is OK
- 10.1145/3296979.3192369 is OK

MISSING DOIs

- Errored finding suggestions for "uthash: a hash table for C structures", please try later
- Errored finding suggestions for "jsmn, a minimalistic JSON parser in C", please try later
- Errored finding suggestions for "fast_double_parser: 4\times faster than \textttstr...", please try later
- Errored finding suggestions for "RyΕ« & RyΕ« Printf", please try later
- Errored finding suggestions for "The Meson Build System", please try later
- Errored finding suggestions for "Ninja, a small build system with a focus on speed.", please try later
- Errored finding suggestions for "Bokeh, Interactive Data Visualization.", please try later
- Errored finding suggestions for "\textsclibcdict examples", please try later
- Errored finding suggestions for "\textscglib hash tables", please try later

INVALID DOIs

- None
pgrete commented 2 years ago

@ivastar as mentioned via mail, I'll first be able to start the review around the second half of October

langmm commented 2 years ago

Review checklist for @langmm

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

langmm commented 2 years ago

Related issue opened on Gitlab: https://gitlab.com/robizzard/libcdict/-/issues/7

pgrete commented 1 year ago

Review checklist for @pgrete

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

pgrete commented 1 year ago

I'm done with the first round of review.

Related issues on GitLab:

In addition to the open "issues" I think that a table of contents (with links) in the Readme.md could be quite useful as I had troubles navigating this long document.

ivastar commented 1 year ago

@langmm and @pgrete thank you for the review! If you have any comments on that were not captured in the issues, please add them here as a comment ASPA.

@robizzard I believe the ball is now in your court. Please address the open issues and let me know when you are finished.

robizzard commented 1 year ago

Hi Ivelina,

It's on my todo list (with a hundred other things, sorry) - will get to it asap.

thanks! Rob

On Thu, 26 Jan 2023 at 12:33, Ivelina Momcheva @.***> wrote:

@langmm https://github.com/langmm and @pgrete https://github.com/pgrete thank you for the review! If you have any comments on that were not captured in the issues, please add them here as a comment ASPA.

@robizzard https://github.com/robizzard I believe the ball is now in your court. Please address the open issues and let me know when you are finished.

β€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4756#issuecomment-1404938008, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGE7BNONIPFHJEFNFYTEFU3WUJVLHANCNFSM6AAAAAAQKMEZYQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

arfon commented 1 year ago

@robizzard @ivastar – this submission is rather stale now. @robizzard – can you commit to completing these changes in the next month? If not, we may have to close this review as stale/abandoned.

ivastar commented 1 year ago

@robizzard please respond in the next week if you are planning to complete these changes.

robizzard commented 1 year ago

hi Ivelina,

I'm hoping to! Sorry, day job is already 60h/week, trying to work around it and stay healthy.

thanks! Rob

On Mon, 16 Oct 2023 at 09:24, Ivelina Momcheva @.***> wrote:

@robizzard https://github.com/robizzard please respond in the next week if you are planning to complete these changes.

β€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4756#issuecomment-1763974662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGE7BNNKPRAT5D25WITR7ELX7TVM7ANCNFSM6AAAAAAQKMEZYQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

robizzard commented 1 year ago

hi Ivelina,

I've fixed everything that was in the issues, which I think is everything required. I've left one open because I wasn't sure about it, and would like the reporter (Philipp Grete https://gitlab.com/pgrete) to confirm I've done what they wanted.

thanks! Rob

On Mon, 16 Oct 2023 at 09:24, Ivelina Momcheva @.***> wrote:

@robizzard https://github.com/robizzard please respond in the next week if you are planning to complete these changes.

β€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4756#issuecomment-1763974662, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGE7BNNKPRAT5D25WITR7ELX7TVM7ANCNFSM6AAAAAAQKMEZYQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

ivastar commented 12 months ago

Dear @langmm and @pgrete. The author has made the requested changes. Would you please review them and let me know if you accept the submission? If you would like to request further changes or have comments, please add them in the issues or here in this thread. If you accept the submission, please comment in this thread to that effect.

langmm commented 12 months ago

Looks good to me, I accept the submission.

pgrete commented 11 months ago

Same here, I also accept the updated submission.

ivastar commented 11 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

ivastar commented 11 months ago

@editorialbot generate pdf

editorialbot commented 11 months ago

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

ivastar commented 11 months ago

@editorialbot check references

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

OK DOIs

- 10.1111/j.1365-2966.2004.07446.x is OK
- 10.1051/0004-6361:20066129 is OK
- 10.1051/0004-6361/200912827 is OK
- 10.1093/mnras/stx2355 is OK
- 10.1002/spe.2984 is OK
- 10.1145/3296979.3192369 is OK
- 10.1145/3360595 is OK
- 10.21105/joss.04642 is OK
- 10.1093/mnras/stac2899 is OK

MISSING DOIs

- None

INVALID DOIs

- None
ivastar commented 11 months ago

Dear @robizzard,

I have a couple of small corrections on the references below. Afterwards, please follow the checklist above - tag a version, do an archive release (make sure authors in the archive release and license are the same as the JOSS paper), and put the info in a comment here. Let me know if you have questions.

Best, -Iva

robizzard commented 10 months ago

hi all,

I think I've done all that's requested, please just let me know if I have missed something.

many thanks, and happy holidays when (if :) you get them

Rob

Double check authors and affiliations (including ORCIDs)

done

Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.

This is release version 1.4

Archive the release on Zenodo/figshare/etc and post the DOI here.

Zenodo DOI: 10.5281/zenodo.10276619

Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.

done

Make sure that the license listed for the archive is the same as the software license.

done (GPLV3)

On line 13, remove "mostly"?

done (wasn't essential, just indicative that libcdict doesn't do everything :)

On line 74, please replace the bokeh citations with the one recommended by the team: https://docs.bokeh.org/en/0.10.0/docs/contributing.html#citation

done

On line 90: the reference to glib looks awkward. Replace documentation with perhaps glib documentation. I recommend a link directly to the docs rather than the hash function https://docs.gtk.org/glib/index.html but your call.

Sadly glib don't provide instructions like Bokeh do :) But happy to change it and let the reader do the hash-function lookup.

On line 92: the second reference to the glib repo looks like a GitLab citation. Maybe just cite the docs from the previous comment?

done

Line 99: cite your project with the DOI

TODO

Lines 125 and 127: I would recommend only citing the repo, the link to the docs is already there. The webpage is privately hosted and may therefore disappear in the future.

done - was just trying to be complete.

I have also similarly removed the jsmn, ninja homepages links, as previously the "privately hosted" issue applies to them too (and I checked the links are on the github/gitlab repo pages).

On Tue, 7 Nov 2023 at 10:37, Ivelina Momcheva @.***> wrote:

Dear @robizzard https://github.com/robizzard,

I have a couple of small corrections on the references below. Afterwards, please follow the checklist above - tag a version, do an archive release (make sure authors in the archive release and license are the same as the JOSS paper), and put the info in a comment here. Let me know if you have questions.

Best, -Iva

  • On line 13, remove "mostly"?
  • On line 74, please replace the bokeh citations with the one recommended by the team: https://docs.bokeh.org/en/0.10.0/docs/contributing.html#citation
  • On line 90: the reference to glib looks awkward. Replace documentation with perhaps glib documentation. I recommend a link directly to the docs rather than the hash function https://docs.gtk.org/glib/index.html but your call.
  • On line 92: the second reference to the glib repo looks like a GitLab citation. Maybe just cite the docs from the previous comment?
  • Line 99: cite your project with the DOI
  • Lines 125 and 127: I would recommend only citing the repo, the link to the docs is already there. The webpage is privately hosted and may therefore disappear in the future.

β€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4756#issuecomment-1798243610, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGE7BNNKGX5AYTYNSFWJBZDYDIFOBAVCNFSM6AAAAAAQKMEZYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJYGI2DGNRRGA . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

ivastar commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

ivastar commented 10 months ago

Hi @robizzard, I'm sorry but a couple of more steps here. Items 2-4 are really go all together, make sure that the title, authors and license of the archive are the same as those in the paper.

  • [ ] Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
  • [ ] Archive the release on Zenodo/figshare/etc and post the DOI here.
  • [ ] Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.
  • [ ] Make sure that the license listed for the archive is the same as the software license.

Then please post the version number and the DOI here.

robizzard commented 10 months ago

Hi Ivelina,

I thought I'd done those above, copy pasting it below.

cheers rob

Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.

This is release version 1.4

Archive the release on Zenodo/figshare/etc and post the DOI here.

Zenodo DOI: 10.5281/zenodo.10276619

Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.

done

Make sure that the license listed for the archive is the same as the software license.

done (GPLV3)

On Wed, 6 Dec 2023 at 14:49, Ivelina Momcheva @.***> wrote:

Hi @robizzard https://github.com/robizzard, I'm sorry but a couple of more steps here. Items 2-4 are really go all together, make sure that the title, authors and license of the archive are the same as those in the paper.

  • Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
  • Archive the release on Zenodo/figshare/etc and post the DOI here.
  • Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.
  • Make sure that the license listed for the archive is the same as the software license.

Then please post the version number and the DOI here.

β€” Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/4756#issuecomment-1843042428, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGE7BNOOQ7HZ2L2NACWFHFDYICAXLAVCNFSM6AAAAAAQKMEZYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGA2DENBSHA . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

ivastar commented 10 months ago

Ah, thanks! I missed them.

ivastar commented 10 months ago

@editorialbot set 10.5281/zenodo.10276619 as archive

editorialbot commented 10 months ago

Done! archive is now 10.5281/zenodo.10276619

ivastar commented 10 months ago

@editorialbot set 1.4 as version

editorialbot commented 10 months ago

Done! version is now 1.4

ivastar commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

ivastar commented 10 months ago

@editorialbot check references

editorialbot commented 10 months ago

Checking the BibTeX entries failed with the following error: 

Failed to parse BibTeX on value "author" (NAME) [#<BibTeX::Bibliography data=[5]>, "@", #<BibTeX::Entry >, "%"]
ivastar commented 10 months ago

@editorialbot check references

editorialbot commented 10 months ago

Checking the BibTeX entries failed with the following error: 

Failed to parse BibTeX on value "author" (NAME) [#<BibTeX::Bibliography data=[5]>, "@", #<BibTeX::Entry >, "%"]
ivastar commented 10 months ago

@dfm this submission is ready for publication but I am not sure how to debug the error above. Please advice.

robizzard commented 10 months ago

I have a couple of comments on the proofs from David (co-author) so will try to address these and get back to you. Will try to fix the above error if I see it!

On Wed, 6 Dec 2023 at 15:07, Ivelina Momcheva @.***> wrote:

@dfm this submission is ready for publication but I am not sure how to debug the error above. Please advice.

β€” Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

dfm commented 10 months ago

@openjournals/dev β€” Can you advise on this error during the "check references" step? I'm not sure how to get the full error. Thanks!

robizzard commented 10 months ago

Hi all,

I've updated the libcdict version to V1.41 after we checked the proofs. DOI: 10.5281/zenodo.10287855 Zenodo: https://zenodo.org/records/10287855

This fixes a typo in the glib citation, and adds a few papers that have come out recently that use libcdict. (thanks to David Hendriks for these)

I'll let your editorial team decide if this is a minor enough change to let it pass - normally it would be.

And regarding the BibTeX error, I don't see that here, but I'm using an old docker to build the paper:

openjournals/inara latest 806f51ef6d31 16 months ago 755MB

... if you would like me to update the docker image and try to reproduce the error message, I can do so, but not tonight!

thanks! Rob

On Wed, 6 Dec 2023 at 15:55, Rob Izzard @.***> wrote:

I have a couple of comments on the proofs from David (co-author) so will try to address these and get back to you. Will try to fix the above error if I see it!

On Wed, 6 Dec 2023 at 15:07, Ivelina Momcheva @.***> wrote:

@dfm this submission is ready for publication but I am not sure how to debug the error above. Please advice.

β€” Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. Robert Izzard - @.***

-- Dr. Robert Izzard - @.***

ivastar commented 10 months ago

@editorialbot commands

editorialbot commented 10 months ago

Hello @ivastar, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Add to this issue's reviewers list
@editorialbot add @username as reviewer

# Remove from this issue's reviewers list
@editorialbot remove @username from reviewers

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Assign a user as the editor of this submission
@editorialbot assign @username as editor

# Remove the editor assigned to this submission
@editorialbot remove editor

# Remind an author, a reviewer or the editor to return to a review after a 
# certain period of time (supported units days and weeks)
@editorialbot remind @reviewer in 2 weeks

# Check the references of the paper for missing DOIs
@editorialbot check references

# Perform checks on the repository
@editorialbot check repository

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for version
@editorialbot set v1.0.0 as version

# Set a value for branch
@editorialbot set joss-paper as branch

# Set a value for repository
@editorialbot set https://github.com/organization/repo as repository

# Set a value for the archive DOI
@editorialbot set set 10.5281/zenodo.6861996 as archive

# Mention the EiCs for the correct track
@editorialbot ping track-eic

# Generates the pdf paper
@editorialbot generate pdf

# Recommends the submission for acceptance
@editorialbot recommend-accept

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Flag submission with questionable scope
@editorialbot query scope

# Get a link to the complete list of reviewers
@editorialbot list reviewers

# Creates a post-review checklist with editor and authors tasks
@editorialbot create post-review checklist

# Open the review issue
@editorialbot start review
ivastar commented 10 months ago

@editorialbot set 1.41 as version

editorialbot commented 10 months ago

Done! version is now 1.41

ivastar commented 10 months ago

@editorialbot set 10.5281/zenodo.10287855 as archive