[REVIEW]: Brightway: An open source framework for life cycle assessment

whedon commented 7 years ago

Submitting author: @cmutel (Chris Mutel) Repository: https://bitbucket.org/cmutel/brightway2 Version: 2.1 Editor: @katyhuff Reviewer: @amoeba Archive: 10.5281/zenodo.556145

Status

Status badge code:

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

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 questions

Conflict of interest

[x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

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?
[ ] Version: Does the release version given match the GitHub release (2.0.2)?
[x] Authorship: Has the submitting author (@cmutel) made major contributions to the software?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: Have any performance claims of the software been confirmed?

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?
[ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[ ] 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?
[ ] 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)?

whedon commented 7 years ago

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @amoeba 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 as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

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

watching

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

amoeba commented 7 years ago

I've just made some time to sit down with this submission, though I am not yet done. Overall, this software is very clearly research software and I imagine this software is useful to researchers. The code is clean, includes documentation and helpful introductions so I commend the author on the hard work putting together that documentation. I've gone through a few of the easy checklist items and have put my notes below:

Repository: Is the source code for this software available at the repository url?

Sort of. @katyhuff please advise: The source code at the linked repository is a meta-package a set of individual packages. From an open source perspective, this is fine. But from a JOSS perspective, I wonder what we'll do about the archival ZIP for the submission. Thoughts?

License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?

Two issues:

LICENSE file named LICENSE.txt. It seems pedantic and unnecessary to ask the author to change the filename so I will ignore this difference unless the editor thinks otherwise.
LICENSE files do not contain the text of an OSI-approved software license. Suggest author choose a license and replaces the contents of these files with the text of the license.

Version: Does the release version given match the GitHub release (2.0.2)?

Code is not on GitHub but BitBucket. I think this is fine but the checklist item is vague as to whether this is a problem. I'll accept this difference and move on.
The BitBucket repository does contain a tag for the 2.0.2 release which is also the latest release of the software
The install instructions install version 2.1 of the software at present. This isn't an issue AFAICT from the JOSS guidelines but as a reviewer I'm wondering why there is a discrepancy here.

References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

No. Of the references used, none have DOIs and all but one reference (the developer's blog) should have DOIs. DOIs for the other software repositories this submission's repository contains relates to my above issue about this submission's code being split across numerous software repositories.

I will continue working on the rest of the checklist items now.

cmutel commented 7 years ago

Thanks @amoeba-

License is, or at least, should be BSD 3-clause, unless I messed up a copy/paste.
2.1 is now the latest tag in repo, as well as the release version
Added DOIs to the paper citations in paper.bib.

You are totally right that this is a "meta repository" - for maintainability and separation of concerns this decision was taken many years ago. No idea how this effects JOSS, but it is certainly the right choice for a large and evolving project.

amoeba commented 7 years ago

Totally agree with you on the meta-repo concept. Hopefully we can find a resolution that fits JOSS.

Re: Licensing, I'm not a lawyer but your License file says

Copyright (c) 2016, Chris Mutel and ETH Zurich All rights reserved.

which doesn't seem to be compatible with the licensing requirements here. My understanding is that the license file would need to only include the BSD license text. Again, not a lawyer.

arfon commented 7 years ago

On the meta-repo concept: I think this is fine as long as the archive (with figshare or Zenodo) includes all of the associated repositories. Does that sound reasonable @amoeba and @cmutel?

amoeba commented 7 years ago

Yep, that sounds ideal.

cmutel commented 7 years ago

Sorry, that was an embarrassing mistake. And of course it was then copy/pasted in a million other places.

License is now fixed for all components, and new releases are on pypi and anaconda.

amoeba commented 7 years ago

I've sat down with the rest of the checklist and made good progress. Again, thanks @cmutel for the extensive documentation and example code. See my comments below:

Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

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

The author has a nicely put-together documentation page at https://docs.brightwaylca.org/ which does include this information. Can the content at that page be included in our archival ZIP? Usually the information fulfilling these three checklist items is contained inside the README(.md) on the source code repository which I think is the best place for such information due to the archive-oriented nature of JOSS. Given the meta-package aspect of this submission that we have already discussed, I understand this may not make as much sense.

I request that the author either update the README files to include this information or archive the content at https://docs.brightwaylca.org with the submission's archive.

Functionality: Have the functional claims of the software been confirmed?

I ran through some of the provided Jupyter notebooks and had no trouble running the code and seeing a reasonable result. After some reading (most the very helpful docs), I have only a vague idea what LCA is. I have no reason to believe the software is not working as intended but I can't say I'd be checking off this item with as much confidence as I have with other JOSS reviews I've done. Please advise @katyhuff ?

Otherwise, everything else is in order and I've checked off some remaining items. As it stands, I request the author review my comments on Installation/Example usage/Community guidelines and that the Version is rectified now that updates have been made since submission to JOSS.

Suggest: Accept with minor revisions and after discussion with @katyhuff about my concerns (above) over the Functionality checklist item (see above comments).

cmutel commented 7 years ago

@amoeba One small note on the documentation and usage examples - both the documentation and usage examples are part of the brightway2 "meta-package." Not sure how this is supposed to work - are the input (restructured text & jupyter notebook) files enough, or would the output (html) files also be needed?

amoeba commented 7 years ago

Thanks for linking those. I think what you've got there is sufficient. The JOSS guidelines are vague as to where such information exists and what form it exists in but I'd say what you've got counts:

From the guidelines:

A high-level overview of this documentation should be included in a README file (or equivalent).

cmutel commented 7 years ago

@katyhuff It seems like the outstanding issues have been addressed - would it be possible to complete this review this week? It would be very nice to be able to have a citation for a proposal due next Monday :)

katyhuff commented 7 years ago

@amoeba Thanks so much for this review. Regarding the funcitonality issue, this does look to be a useful framework for LCA and meets the functionality claims that are made.

@cmutel Thanks for addressing the comments quickly. At this point could you make an updated archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the version 2.1 archive? I can then move forward with accepting the submission!

cmutel commented 7 years ago

@katyhuff I uploaded the complete archive to Zenodo: https://zenodo.org/record/556145

DOI is 10.5281/zenodo.556145

arfon commented 7 years ago

@whedon set 10.5281/zenodo.556145 as archive

whedon commented 7 years ago

OK. 10.5281/zenodo.556145 is the archive.

arfon commented 7 years ago

Thanks for your review @amoeba and for editing this one @katyhuff

@cmutel - your paper is now accepted into JOSS and your paper DOI is http://dx.doi.org/10.21105/joss.00236 ⚡️ 🚀 💥

cmutel commented 7 years ago

Many thanks to all involved!

katyhuff commented 7 years ago

Thanks @cmutel @amoeba and @arfon !

openjournals / joss-reviews