Joss-reviews: [REVIEW]: KLRfome - Kernel Logistic Regression on Focal Mean Embeddings

[whedon]

Submitting author: @mrecos (Matthew Harris)
Repository: https://github.com/mrecos/klrfome
Version: v2.2.0
Editor: @arfon
Reviewer: @benmarwick
Archive: 10.5281/zenodo.2598673

Status

Status badge code:

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

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) 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

@benmarwick, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

1. Make sure you're logged in to your GitHub account
2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @leeper know.

Review checklist for @benmarwick

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: v2.2.0
• [x] Authorship: Has the submitting author (@mrecos) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  BM: I think Szabo should be noted in the readme as someone who made a contribution that made this pkg possible
  BM: done

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

The package includes all the following forms of documentation:

• [x] A statement of need clearly stating problems the software is designed to solve and its target audience in README
  BM: very nicely stated.
• [x] Installation instructions: for the development version of package and any non-standard dependencies in README
• [x] Vignette(s) demonstrating major functionality that runs successfully locally
  BM: There is no vignette, but the README gives a very thorough walk-through. I think I've seen a method to have a vignette, and auto-extract it into the README on pkg build, that would help to fulfil this requirement.
  BM: done
• [x] Function Documentation: for all exported functions in R help
  BM: documentation is present, but uneven. Descriptions and examples are missing for many functions. These should be added. Spelling: scaler should be scalar. The KLR function has a bit of commented-out code in it that probably should be removed.
  BM: done
• [x] Examples for all exported functions in R Help that run successfully locally
  BM: not yet, though the readme is a good start.
  BM: done
• [x] Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).
  BM: There is no 'Maintainer' field in description. The pkg referred to in the CONTRIBUTING is DistRegLMERR, presumably an earlier name for this pkg, but should be changed.
  BM: done

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Additional criteria (from rOpenSci)

BM: I pasted these over from the rOpenSci onboarding

• [x] Packaging guidelines: The package conforms to the rOpenSci packaging guidelines: https://github.com/ropensci/onboarding/blob/master/packaging_guide.md
  BM: not yet, still some documentation to add. There is no NEWS file. There are no tests.
  BM: done
  
  
  
  • [x] Automated tests: Unit tests cover essential functions of the package
    
    and a reasonable range of inputs and conditions. All tests pass on the local machine.
    
    BM: not yet.
    
    BM: done
  
  

#### Final approval (post-review)

• [x] The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

  Estimated hours spent reviewing:
  3

---

### Review Comments
The author has done an excellent job of fulfilling the requirements for JOSS. In particular, it's an excellent example of superb package documentation to the point that I believe any undergraduate archaeology student (or related field) should be able to make sense of what is the need and purpose, and quickly start using the pkg. The vignette is outstanding and deserves a wide audience. Well done!

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @benmarwick it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

--> Check article proof :page_facing_up: <--

[leeper]

@benmarwick I can see you've made some progress on this review. Can you open issues over at the source repo for any issues you'd like to see addressed? Thanks!

[benmarwick]

Righto, thanks for the tip, I've opened a few relevant issues now.

[leeper]

@benmarwick Great. Thanks! Let us know if you have any other comments at this point.

@mrecos Please try to address the issues opened by @benmarwick. The matters of examples and tests are particularly important.

[mrecos]

Great! Thanks @benmarwick and @leeper. I will address in short order and report back.

[leeper]

@mrecos Just wanted to check in. Are you still planning to make revisions for this?

[mrecos]

Hello @leeper , yes I am. My apologies. I will work on it this coming week.

[leeper]

@mrecos Just another nudge on this.

[mrecos]

Thanks @leeper - Making edits/additions/commits now.

[arfon]

@mrecos - how are you getting along here?

[arfon]

@mrecos - did you manage to make your revisions?

[arfon]

Just heard back from the author (via email) that he's still interested in pursuing this publication with JOSS and is working on changes as a result of the review.

[arfon]

@whedon assign @arfon as editor

[mrecos]

Hello @afron. Quick update: I took care of 2 of 3 issues on my repo. TODO - convert existing how-to to vignette and unit tests.

[arfon]

:wave: @mrecos - how are you getting along here?

[mrecos]

Hello @arfon . Going well. I took care of all points except for the unit tests. I am currently implementing those.

[mrecos]

Hello @arfon . I have completed the code updates requested by Ben via his review and Issues. These include general code cleanup, documentation of all public functions, a vignette, and unit testing. Please review and advise. Thanks!

[arfon]

Hello @arfon . I have completed the code updates requested by Ben via his review and Issues. These include general code cleanup, documentation of all public functions, a vignette, and unit testing. Please review and advise. Thanks!

Great!

@benmarwick - when you get a chance, could you please come and take another look?

[labarba]

👋 @benmarwick — It looks like we are waiting for you to check the author's responses to your comments. What's your status?

[benmarwick]

Thanks for the reminder. I've had another look at the package and everything looks great! The author has done an excellent job of fulfilling the requirements for JOSS. In particular, it's an excellent example of superb package documentation to the point that I believe any undergraduate archaeology student (or related field) should be able to make sense of what is the need and purpose, and quickly start using the pkg. The vignette is outstanding and deserves a wide audience. Well done!

[labarba]

I see some unchecked items in the checklist—if you're satisfied, can you tick off those items?

[benmarwick]

Done!

[arfon]

@whedon accept

[whedon]

No archive DOI set. Exiting...

[arfon]

@mrecos - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

[mrecos]

Hello @arfon - I create a the "JOSS Release" and committed to Zenodo for a new DOI (10.5281/zenodo.2598673). I checked the DOI badge on the KLRfome repo and all seems well. Let me know if this covers it. Thanks!

[labarba]

@whedon set 10.5281/zenodo.2598673 as archive

[whedon]

OK. 10.5281/zenodo.2598673 is the archive.

[labarba]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[labarba]

Can you edit the metadata of the Zenodo deposit (no need to get new version or new DOI) so the title and author list match the JOSS paper?

[labarba]

@whedon set v2.2.0 as version

[whedon]

OK. v2.2.0 is the version.

[mrecos]

Hello @labarba, ok updated! I had found a typo in the release version of the JOSS paper markdown just now. I pushed the edits to the repo, but they are now post 2.2.0 release. Should I do redo the release?
https://zenodo.org/record/2598673#.XJE_3FVKipo

[labarba]

If it's just an edit on the JOSS paper, I don't see a need to redo the release (the final JOSS paper is archived on its own in the journal).

[mrecos]

ok, then we should be good to go. Please let me know how else I can help.

[labarba]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/561

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/561, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

[whedon]

```Reference check summary:

OK DOIs

• None

MISSING DOIs

• https://doi.org/10.1198/106186005x25619 may be missing for title: Kernel logistic regression and the import-vector machine
• https://doi.org/10.1561/2200000060 may be missing for title: Kernel Mean Embedding of Distributions: A Review and Beyond
• https://doi.org/10.1109/msp.2013.2252713 may be missing for title: Kernel Embeddings of Conditional Distributions: A Unified Kernel Framework for Nonparametric Inference in Graphical Models

INVALID DOIs

• None
  ```

[labarba]

Please check if the suggested "Mising DOIs" are indeed missing, or are flukes of whedon's Crossref query.

[labarba]

@arfon — Are you OK with the final version of the paper?

@mrecos — This is also your final chance to check the proof, and correct any typos / grammar / punctuation / author name spelling / reference titles needing cap protection ...

[mrecos]

@labarba whedon was correct, those citations had DOIs to include. I commit/pushed a new paper.bib with the DOIs included. Aside from that, the proof looks good!

[arfon]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

:point_right: Check article proof :page_facing_up: :point_left:

[mrecos]

@whedon accept deposit=true

[whedon]

I'm sorry @mrecos, I'm afraid I can't do that. That's something only editor-in-chiefs are allowed to do.

[arfon]

The KLRfome package is available at https://github.com/mrecos/klrfome and DOI https://doi.org/10.5281/zenodo.1218403

@mrecos - could you please remove this sentence from your paper? We link to the repository and archive DOI in the paper (in the left-hand margin).

[mrecos]

@arfon ok, sentence removed, committed, and pushed.

[arfon]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

```Reference check summary:

OK DOIs

• 10.1198/106186005x25619 is OK
• 10.1561/2200000060 is OK
• 10.1109/msp.2013.2252713 is OK

MISSING DOIs

• None

INVALID DOIs

• None
  ```

[whedon]

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/562

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/562, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

[arfon]

@whedon accept deposit=true

[whedon]

Doing it live! Attempting automated processing of paper acceptance...

[whedon]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/563
2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.00722
3. If everything looks good, then close this review issue.

4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

   Any issues? notify your editorial technical team...

[arfon]

@benmarwick - many thanks for your review here ✨

@mrecos - your paper is now accepted into JOSS :zap::rocket::boom:

[whedon]

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.00722/status.svg)](https://doi.org/10.21105/joss.00722)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.00722">
  <img src="http://joss.theoj.org/papers/10.21105/joss.00722/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.00722/status.svg
   :target: https://doi.org/10.21105/joss.00722

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=

[mrecos]

Thank you, Thank you, Thank you! @arfon , @labarba , @benmarwick , @leeper, and of course @whedon
You are all incredibly helpful and I greatly appreciate your respective efforts.
Cheers!