Joss-reviews: [REVIEW]: Pymer4: Connecting R and Python for Linear Mixed Modeling

Created on 30 Jul 2018  ยท  42Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @ejolly (Eshin Jolly)
Repository: https://github.com/ejolly/pymer4
Version: 0.5.0
Editor: @cMadan
Reviewers: @janfreyberg, @tomfaulkenberry
Archive: 10.5281/zenodo.1523205

Status

status

Status badge code:

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

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

@janfreyberg & @tomfaulkenberry, 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 @cMadan know.

โœจ Please try and complete your review in the next two weeks โœจ

Review checklist for @janfreyberg

Conflict of interest

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: Does the release version given match the GitHub release (0.5.0)?
  • [x] Authorship: Has the submitting author (@ejolly) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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

  • [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] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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

Review checklist for @tomfaulkenberry

Conflict of interest

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: Does the release version given match the GitHub release (0.5.0)?
  • [x] Authorship: Has the submitting author (@ejolly) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

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

  • [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] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

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)?
accepted published recommend-accept review

Most helpful comment

Whoops fixed! Woot thanks much @cMadan, @janfreyberg, @tomfaulkenberry for your helpful feedback. This was a great review experience.

All 42 comments

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @RMHogervorst, 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:

watching

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

notifications

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

@whedon commands
Attempting PDF compilation. Reticulating splines etc...

Proof looks good to me. Thanks for agreeing to review all.

@RMHogervorst, @janfreyberg, @tomfaulkenberry - it would be great if you could start reviewing soon. Please let me know if you have any questions about the process or journal policy.

Dear @cMadan and @ejolly ,

It takes an extraordinary amount of effort for me to install the package let alone test it on my ubuntu computer.It seems I have to do a lot of python magic to make it work. I do not have the time nor energy to finish this review in 2 weeks and would like to quit the review. I'm sorry for the inconvenience.

Hi @cMadan and @ejolly:

I am currently traveling with family for a little vacation before our Fall term starts. When I get back to work later next week, I will immediately go through the paper and software and finish my review. From what I've seen in my initial look, this shouldn't take long :)

Hi @cMadan and @ejolly - just raised a few issues on the repo, but it's a well-documented, well-running package, so my issues are fairly minor:
https://github.com/ejolly/pymer4/issues/28
https://github.com/ejolly/pymer4/issues/29
https://github.com/ejolly/pymer4/issues/30

Nice work!

@RMHogervorst, I am fine with you taking longer than two weeks if that would be more practical, but if you feel that reviewing this submission still is not feasible, then no worries about withdrawing as a reviewer. I do think, however, that if the package is too difficult for some to install, that is something we should try and improve in the review process, even if it means you focus on that one aspect rather than reviewing the submission as a whole (since we do have other reviews that can cover that side of things).

@tomfaulkenberry, not a problem, enjoy your vacation!

@janfreyberg, thanks for your work on the review so far!

Overall, I found the paper to be concise, well-written, and perfectly describes to the reader why and how to use the software. Further, I found the repo documentation to be equally well-written. I did not run into any issues with the software. There is ONE typo in the example code in the paper: on the line that begins

model = Lmer('DV'=

there is a missing parenthesis at the end of the line (it took me a while to figure out that this was the issue!).

Overall, well done on both the software and the paper!

@cMadan I believe I have addressed all the issues brought up so far, but there's one in particular (https://github.com/ejolly/pymer4/issues/28) opened by @janfreyberg, and related to the installation issues @RMHogervorst was running into, that I would really appreciate some advice on. In summary, I'm not sure whether to favor support for both Python 2 and 3, or drop Python 2 support but make overall installation a bit easier for some users. Rather than reiterating it all here, I've noted my question in that issue thread. Thanks!

While supporting both Python 2 and 3 is admirable, at some point you will have to drop Python 2 support since that's inevitable and happing fast (e.g., see https://python3statement.org). If there are issues currently in trying to remain cross-compatible, my inclination is to stick with just Python 3 already--and then streamline the install experience for that procedure. Ultimately the decision is yours of course, but that's the way I see it.

@ejolly, how are things going?

Hey @cMadan sorry this has a been a bit frustrating to sort out various installation issues but I think thinks should be set for the most part now! The trickiness has really been with rpy2 a core dependency of pymer4. Despite much effort from their development team it appears to me that it's relatively well known in the community that installation of that pacakage can be tricky. For compatibility going forward I made sure that pymer4 now no longer depends on a specific (older) version of rpy2 which was causing a lot of problems. It is compatible with the most current version of rpy2 (which itself is more bug free) and should support all non-API breaking future versions.

I just pushed a new version to pypi and thoroughly updated the README and documentation site with two different installation options along with an installation tester function. The first should be much easier for most Python users who already use the Anaconda distribution. This was suggested by @janfreyberg 's review in https://github.com/ejolly/pymer4/issues/28. I've also expanded some instructions for an alternative installation method for R or Python users that don't use Anaconda. I've done the best I can to support the most common issues that users may encounter and have tested both installation methods myself on numerous machines. As per your suggestion "dropping" Python 2 support in this new version helped a bit; in many cases Python 2 _should_ work I'm just not adding official support for it in versions >= 0.6.0. Users still can install 0.5.0 if they are insistent on Python 2.

Please let me know if there's anything else I need to do for the review process!

@ejolly, thanks for the detailed reply, I appreciate that this was a difficult issue to work through. I think you're almost all set here though!

@tomfaulkenberry and @janfreyberg, can you take a look and see if the latest changes satisfy your suggestions?

@RMHogervorst, I know you had issues with the installation before and weren't able to test the package fully. Do you want to give it another try now? Since we have two reviewers who are almost all done, I don't think we need a third here, but do let me know if you're still interested in reviewing this submission.

This looks good to me and the new installation instructions are very good! I like the test_install() function, too.

@janfreyberg, I assume you're happy to check those last three boxes in the reviewer guidelines?

done!

@janfreyberg, thank you for your thorough review, it is much appreciate!

@RMHogervorst, I am going to remove you as an assigned reviewer here, but I appreciate your feedback with the installation issues earlier on. Hopefully the changes to the project have now ameliorated the issues you were having.

@ejolly, I will do a final look over of the project in the next few days and get back to you.

@whedon remove @RMHogervorst as reviewer

OK, @RMHogervorst is no longer a reviewer

@ejolly, everything looks good, just two things to do:

  • update the pip link in your readme, currently it's set for v. 0.2.2 ([![Package versioning](https://img.shields.io/pypi/v/pymer4.svg)](https://pypi.python.org/pypi?name=pymer4&version=0.2.2&:action=display))
  • mint a release for 0.6.0 and tell me the Zenodo DOI for the archived code

@cMadan Done and done. The readme contains the correct badge link for both. Here is the latest DOI. Please let me know if you need anything else!

@whedon set 10.5281/zenodo.1523205 as archive

OK. 10.5281/zenodo.1523205 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

PDF failed to compile for issue #862 with the following error:

% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 12 0 12 0 0 178 0 --:--:-- --:--:-- --:--:-- 179
Error reading bibliography ./paper.bib (line 2, column 4):
unexpected "t"
expecting space or ","
Error running filter pandoc-citeproc:
Filter returned error status 1
Looks like we failed to compile the PDF

@ejolly, looks like your bib is missing a comma at the end of line 1.

Otherwise you're all set for the submission to be accepted!

Whoops fixed! Woot thanks much @cMadan, @janfreyberg, @tomfaulkenberry for your helpful feedback. This was a great review experience.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon accept

Attempting dry run of processing paper acceptance...

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

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/80, 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, can you do the honors?

@whedon accept deposit=true

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

๐Ÿšจ๐Ÿšจ๐Ÿšจ 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/81
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.00862
  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...

@janfreyberg, @tomfaulkenberry - many thanks for your reviews here and to @cMadan for editing this submission โœจ

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

: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.00862/status.svg)](https://doi.org/10.21105/joss.00862)

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

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

This is how it will look in your documentation:

DOI

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:

Was this page helpful?
0 / 5 - 0 ratings