search

openjournals / joss-reviews

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

[REVIEW]: PyEI: A Python package for ecological inference #3397

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @karink520 (Karin Knudson) Repository: https://github.com/mggg/ecological-inference Version: v0.1.1 Editor: @drvinceknight Reviewer: @matt-graham, @pmyteh Archive: 10.5281/zenodo.5245632

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@matt-graham & @pmyteh, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @drvinceknight 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 ✨

Review checklist for @matt-graham

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @pmyteh

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @matt-graham, @pmyteh it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

: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

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.07 s (348.7 files/s, 255310.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          12            459            945           1975
Jupyter Notebook                 6              0          14086            451
TeX                              1             19              0            137
Markdown                         2             53              0            116
YAML                             1              4              2             25
Bourne Shell                     3             11              0             21
-------------------------------------------------------------------------------
SUM:                            25            546          15033           2725
-------------------------------------------------------------------------------

Statistical information for the repository '95325e035168347ecb2bdc86' was
gathered on 2021/06/23.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Amy Becker                       2            49             17            0.88
Colin Carroll                   13           358            252            8.12
Gabe Schoenbach                 30           693            269           12.81
JN                               1            17             10            0.36
Karin Knudson                   96          3957           1369           70.94
Matthew Sun                     16           357            149            6.74
karink520                        6             7              4            0.15

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Amy Becker                   28           57.1          1.6                3.57
Colin Carroll               200           55.9          3.8                9.00
Gabe Schoenbach             505           72.9          1.7                3.96
JN                           11           64.7          0.3                0.00
Karin Knudson              2405           60.8          4.5                8.32
Matthew Sun                 230           64.4          8.7                9.13
whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.2307/2088121 is OK
- 10.21105/joss.01143 is OK
- 10.1093/acprof:oso/9780198515326.001.0001 is OK
- 10.1201/b10905-7 is OK

MISSING DOIs

- 10.3886/icpsr01132.v1 may be a valid DOI for title: A solution to the ecological inference problem: Reconstructing individual behavior from aggregate data

INVALID DOIs

- None
whedon commented 3 years ago

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

whedon commented 3 years ago

:wave: @pmyteh, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @matt-graham, please update us on how your review is going (this is an automated reminder).

matt-graham commented 3 years ago

I have added the bulk of my review as issues on the project repository; most of these should only require small fixes. The main substantive issues I would say are some possible improvements to the API documentation (mggg/ecological-inference#62) and adding some discussion of MCMC convergence issues to the notebooks (mggg/ecological-inference#63).

I think overall this submission has a lot of strong points. It seems like a nicely designed Python package which provides a useful contribution to the Python ecosystem by increasing the ease of performing (Bayesian) ecological inference. The provided example notebooks are a great resource for understanding how to use the package to perform inference and create visualisations of the results. The code has decent test coverage, with continuous integration via Github Actions. The paper provides a good summary of the need for the software and its functionality, and gives pointers to comparable packages in R. As far as I can tell from some brief searching there are no other comparable Python packages which have been omitted, or similar packages from other languages such as MATLAB or Julia. The stated functionality all seems to be present in the software.

One general question I have is about the list of authors in the paper. The JOSS reviewer guidelines request that reviewers check that the list of authors appears complete and raise a question if this is not obviously the case. From looking through the commit history it seems that @ColCarroll has made non-negligible contributions to the codebase. Although they are included in the acknowledgements it feels they could also validly be included as an author. Can I check therefore if @ColCarroll has been consulted about authorship?

(Edited to remove specific reference to HTML API documentation issue #60 as I do not consider this a blocker to acceptance)

ColCarroll commented 3 years ago

Hey @matt-graham -- the authors talked to me beforehand. My contributions were around testing/CI (which got me on the commit history!), as well as advising on some computation and API issues, and I am delighted to be in the acknowledgements. Thank you for checking on it, too!

matt-graham commented 3 years ago

@ColCarroll - thanks for the clarification!

pmyteh commented 3 years ago

This is an excellent submission for JOSS. The package meets a real need (ecological inference within the Python ecosystem) and is well executed. The design seems sound, the tutorial and example notebooks are very welcome, and the code is clean and consistent. The paper gives a good overview and is engagingly written. Kudos to the authors.

@matt-graham has already identified most of the issues I spotted. I have +1'd a number of key issues raised by him, and added comments to a couple of them. I've also opened two new issues: one trivial and one more important (though probably rare in practice and very easy to fix). My only other nice-to-have would be to add comments to suppress pylint for the two remaining not-context-manager false-positives found by lint-and-test.py (as well as fixing it to actually trigger the tests as noted in issue 64).

Again - well done. And thanks for giving me a chance to review it.

pmyteh commented 3 years ago

This looks great (some of the issues are still open even though fixes have been merged). Happy to recommend acceptance.

matt-graham commented 3 years ago

Agreed - I am also happy to recommend to accept. Thanks @karink520 for all your work in fixing the issues we raised!

karink520 commented 3 years ago

@matt-graham , @pmyteh , thank you both so much for your thorough and helpful feedback on PyEI!

karink520 commented 3 years ago

@drvinceknight, is there anything I should do at this point? Thank you!

drvinceknight commented 3 years ago

I just have a few last things to check including a last proof read and then I'll ask you to do a new release but we're almost there @karink520 :)

drvinceknight commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

drvinceknight commented 3 years ago

@whedon check references

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

OK DOIs

- 10.2307/2088121 is OK
- 10.21105/joss.01143 is OK
- 10.1093/acprof:oso/9780198515326.001.0001 is OK
- 10.1201/b10905-7 is OK

MISSING DOIs

- 10.3886/icpsr01132 may be a valid DOI for title: A solution to the ecological inference problem: Reconstructing individual behavior from aggregate data

INVALID DOIs

- None
drvinceknight commented 3 years ago

@karink520 could you take a look at the missing DOIs? Whedon has suggested one of them but there are others that are missing:

karink520 commented 3 years ago

@drvinceknight Thanks! All the references we could find DOIs for are now updated. (The one that Whedon suggested has not been updated, since the suggestion is the DOI for an associated dataset, but I don't believe there is a DOI for the book itself).

drvinceknight commented 3 years ago

That's great, thank you.

drvinceknight commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

drvinceknight commented 3 years ago

@whedon check references

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

OK DOIs

- 10.2307/2088121 is OK
- 10.1111/j.1467-985X.2008.00551.x is OK
- 10.1177/0049124199028001004 is OK
- 10.1111/j.1467-985x.2004.02046.x is OK
- 10.1111/1467-9574.00162 is OK
- 10.21105/joss.01143 is OK
- 10.7717/peerj-cs.55 is OK
- 10.1093/acprof:oso/9780198515326.001.0001 is OK
- 10.1201/b16018 is OK
- 10.1201/b10905-7 is OK

MISSING DOIs

- 10.3886/icpsr01132 may be a valid DOI for title: A solution to the ecological inference problem: Reconstructing individual behavior from aggregate data

INVALID DOIs

- None
drvinceknight commented 3 years ago

since the suggestion is the DOI for an associated dataset, but I don't believe there is a DOI for the book itself).

Could you double check if this is the DOI for the book itself (it's unclear to me if it's for the book or the chapter or just completely incorrect all together): https://doi.org/10.1515/9781400849208.343

karink520 commented 3 years ago

@drvinceknight Thank you! I had missed that I guess, but it does look correct to me. Added!

drvinceknight commented 3 years ago

Ok great! Apologies for the delay in getting back to you, I'm just back from some leave. Looking through now.

drvinceknight commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

drvinceknight commented 3 years ago

Could you make a tagged release and archive, and report the version number and archive DOI here please.

Please make sure the archive has the correct metadata: ie that the title and author list match the paper.

karink520 commented 3 years ago

@drvinceknight Done! Version number: v0.1.1. DOI: DOI: 10.5281/zenodo.5245632

drvinceknight commented 3 years ago

@whedon set 10.5281/zenodo.5245632 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.5245632 is the archive.

drvinceknight commented 3 years ago

@whedon set v0.1.1 as version

whedon commented 3 years ago

OK. v0.1.1 is the version.

drvinceknight commented 3 years ago

@whedon recommend-accept

whedon commented 3 years ago 
Attempting dry run of processing paper acceptance...
whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.2307/2088121 is OK
- 10.1111/j.1467-985X.2008.00551.x is OK
- 10.1515/9781400849208 is OK
- 10.1177/0049124199028001004 is OK
- 10.1111/j.1467-985x.2004.02046.x is OK
- 10.1111/1467-9574.00162 is OK
- 10.21105/joss.01143 is OK
- 10.7717/peerj-cs.55 is OK
- 10.1093/acprof:oso/9780198515326.001.0001 is OK
- 10.1201/b16018 is OK
- 10.1201/b10905-7 is OK

MISSING DOIs

- None

INVALID DOIs

- None
whedon commented 3 years ago

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published.

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

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

@whedon accept deposit=true
karink520 commented 3 years ago

Wonderful! Thanks again, all!

karink520 commented 3 years ago

Er, excuse my confusion, but am I the one who should run that last compile with deposit=true ? Or that instruction is aimed at someone else?

danielskatz commented 3 years ago

No, I will do that as the AEiC on duty this week after I proofread the paper and check a few others things.

danielskatz commented 3 years ago

@whedon accept deposit=true

whedon commented 3 years ago 
Doing it live! Attempting automated processing of paper acceptance...
whedon commented 3 years ago

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

whedon commented 3 years ago

🚨🚨🚨 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/2535
  2. Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.03397
  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...

danielskatz commented 3 years ago

Congratulations to @karink520 (Karin Knudson) and co-authors!!

And thanks to @drvinceknight for editing, and @matt-graham and @pmyteh for reviewing! We couldn't do this without you!

whedon commented 3 years ago

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

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

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

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: