[REVIEW]: ChainConsumer about joss-reviews HOT 14 CLOSED

✋ I will review this.

from joss-reviews.

Sorry, I have fallen down on getting this done.

from joss-reviews.

/ cc @openjournals/joss-reviewers - would anyone be willing to review this submission?

If you would like to review this submission then please comment on this thread so that others know you're doing a review (so as not to duplicate effort). Something as simple as :hand: I am reviewing this will suffice.

Reviewer instructions

• Please work through the checklist at the start of this issue.
• If you need any further guidance/clarification take a look at the reviewer guidelines here http://joss.theoj.org/about#reviewer_guidelines
• Please make a publication recommendation at the end of your review

Any questions, please ask for help by commenting on this issue! 🚀

from joss-reviews.

Excellent, thanks Thomas. I had a request for a another feature, so the current version has increased. Hope that isn't an issue.

from joss-reviews.

• The current release on github is 0.10.0, how can the submission be updated?
• I suggest making the installation instruction easier to find (give them a heading). The setup.py should list install_requires. Did not automatically install statsmodels
  • no clear link to contributing guidelines
  • statement of need in the documentation could be expanded (what does corner not do that this covers?). The version in the paper is much better.
  • software archive missing from top of pdf
  • should deps other than mpl get citations?
  • what happens in add_chain with dict input + a list of parameters?

I recommend publication on the condition of addressing the above minor issues.

from joss-reviews.

The current release on github is 0.10.0, how can the submission be updated?

I can update this.

I recommend publication on the condition of addressing the above minor issues.

Many thanks @tacaswell. @Samreay - please let us know on this thread when you've had a chance to review and incorporate @tacaswell's feedback.

from joss-reviews.

Hi guys, am making the requested changes now. In regards to

The setup.py should list install_requires. Did not automatically install statsmodels

Updated.

no clear link to contributing guidelines

Have added a contribution heading with explicit details on the main page and the readme.

statement of need in the documentation could be expanded

Have updated the introduction on the documentation page.

software archive missing from top of pdf

Neither the example in the about page nor the example example page have the software archive shown at the top of the page - it is instead linked inline in the example. I also do not know how to generate the pdf itself, so am unsure what syntax to use. The identifier tag in the paper.json which contains the DOI is already filled out with the zenodo link, I thought that was going to be automatically injected.

should deps other than mpl get citations?

Have added citations for numpy and scipy.

what happens in add_chain with dict input + a list of parameters?

Have updated the API to be clear on this.

from joss-reviews.

software archive missing from top of pdf

@tacaswell @Samreay - yes this is automatically injected at the end of the review once the submission review process is complete. Sorry for any confusion caused here.

Thanks for the rapid turnaround @Samreay. @tacaswell - would you mind taking another look to see if this is good to accept now?

from joss-reviews.

Statsmodels is not directly cited, where as the other 3 direct dependencies are. I defer to @arfon on if that is a blocking issue or not.

otherwise all of my concerns are addressend and looks good to publish 👍

the version needs to be bumped to 0.10.1

from joss-reviews.

Yes, I hope to replace statsmodels with scikit-learns kde implementation in
their next release, so because of that, and statsmodels not having a
publicised bibtex citation (would have to @misc ref their github). In
addition, unlike matplotlib, numpy and scipy, which are used in all
instances of the library usage, the kde is only used on specific request
(which should be rare). For those reasons I left statsmodels out of the
citations. Would be happy to add them in, but would then have to update the
paper.md after the review when this changed, which seems also inappropriate.

Cheers

On Sun, Aug 28, 2016 at 1:05 PM, Thomas A Caswell [email protected]
wrote:

Statsmodels is not directly cited, where as the other 3 direct
dependencies are. I defer to @arfon https://github.com/arfon on if that
is a blocking issue or not.

otherwise all of my concerns are addressend and looks good to publish 👍

the version needs to be bumped to 0.10.1

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#45 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGb5Wf_OphPtyttD8PYRjFMV11S2YHT3ks5qkPr3gaJpZM4JW3Q2
.

from joss-reviews.

Would be happy to add them in, but would then have to update the
paper.md after the review when this changed, which seems also inappropriate.

Provided this is the only change I think we should add Statsmodels to the current list of citations. @Samreay - could you please do this and then 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.

from joss-reviews.

Have added statsmodels and archived. Final doi is
http://dx.doi.org/10.5281/zenodo.61064

On Mon, Aug 29, 2016 at 1:35 AM, Arfon Smith [email protected]
wrote:

Would be happy to add them in, but would then have to update the
paper.md after the review when this changed, which seems also
inappropriate.

Provided this is the only change I think we should add Statsmodels to the
current list of citations. @Samreay https://github.com/Samreay - could
you please do this and then 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.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#45 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGb5WQKuxrV7lnx69vnRdlxidoj1j8EUks5qkaregaJpZM4JW3Q2
.

from joss-reviews.

OK thanks @Samreay. This is now accepted into JOSS and your DOI is http://dx.doi.org/10.21105/joss.00045 🚀 🎉 💥

Thanks for the review @tacaswell ⚡

from joss-reviews.

Thanks guys!

On Wed, Aug 31, 2016 at 1:18 AM, Arfon Smith [email protected]
wrote:

Closed #45 #45.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#45 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGb5WVlO9DfNEICMrreg4xnmpdYbtaLzks5qlEnfgaJpZM4JW3Q2
.

from joss-reviews.