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]: OnlineStats.jl: A Julia package for statistics on data streams #1816

Closed whedon closed 4 years ago

whedon commented 5 years ago

Submitting author: @joshday (Josh Day) Repository: https://github.com/joshday/OnlineStats.jl Version: v1.0.3 Editor: @karthik Reviewers: @pkofod, @ahwillia Archive: 10.5281/zenodo.3659245

Status

status

Status badge code:

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

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

@pkofod & @ahwillia, 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 @karthik know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @pkofod

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @ahwillia

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 5 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @vchuravy, @ahwillia it looks like you're currently assigned to review 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

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

@whedon generate pdf
whedon commented 5 years ago 
Attempting PDF compilation. Reticulating splines etc...
karthik commented 5 years ago

Editor note: @ahwillia will start review in November.

whedon commented 5 years ago

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

ahwillia commented 5 years ago

This looks good to me -- I went ahead and checked all my boxes. The documentation is really nice, and the functionality of the package is clear.

My only feedback is that it would be nice to show more full-fledged examples or tutorials, since the operations performed by this particular package are nice building blocks for more complex models/applications. Are there any examples of other repos using OnlineStats.jl to do impressive things? Can these be mentioned in the documentation or the README?

Other than this suggestion, I am happy to see this accepted and published as is.

ahwillia commented 5 years ago

Also one area the documentation could use more work is the StatLearn object (e.g. for online fitting of GLMs). An example showing how to take multiple passes over the same dataset (i.e. multiple epochs) could be useful.

karthik commented 5 years ago

@vchuravy Quick ping to check in. Can you give us a sense for when you might complete this review? :pray:

karthik commented 5 years ago

/ooo November 21 until December 2

ooo[bot] commented 5 years ago

:+1: Marked @karthik as OOO from Thursday, November 21st 2019 to Monday, December 2nd 2019. :calendar:

karthik commented 4 years ago

@vchuravy Another ping to check in. Can you give us a sense for when you might complete this review?

karthik commented 4 years ago

@vchuravy Pinging one more time. Are you still able to complete the review?

karthik commented 4 years ago

@joshday Apologies for the delays with this submission. I'm still trying to get a response from reviewer 2. I'll look for a reviewer in the new year. I'll be out of office for the next few weeks.

karthik commented 4 years ago

/ooo December 22 2019 until January 10 2020

ooo[bot] commented 4 years ago

:+1: Marked @karthik as OOO from Sunday, December 22nd 2019 to Friday, January 10th 2020. :calendar:

karthik commented 4 years ago

@joshday I am still looking for reviewers without much success. Are you able to recommend anyone that can review without any conflict of interest?

pkofod commented 4 years ago

@karthik I can review but I do have to disclose that we are both employed by Julia Computing. Though, we do live 7000km from each other, and have never worked on projects together. I have never used OnlineStats.jl either, although I'm aware of it's existence.

karthik commented 4 years ago

@pkofod That's totally fine! In fact I've tried to get a few people from Julia Computing to review. Thanks so much for stepping in, assigning you now.

karthik commented 4 years ago

@whedon remove @vchuravy as reviewer

whedon commented 4 years ago

OK, @vchuravy is no longer a reviewer

karthik commented 4 years ago

@whedon assign @pkofod as reviewer

whedon commented 4 years ago

OK, @pkofod is now a reviewer

karthik commented 4 years ago

@pkofod I have updated the checklist above with your name. Please work through that list. Reviewer guidelines are here: http://joss.theoj.org/about#reviewer_guidelines

pkofod commented 4 years ago

I have looked at the repo for the first part and everything looks fine. It's structured like a typical Julia package, and has docs, CI, a proper readme with examples and a clear indication that contributions are welcome but Josh has strong preference for the design choices. I will try to find time to read the actual paper tomorrow or the day after.

pkofod commented 4 years ago

Everything looks pretty good. There is one thing missing IMO, but it might be a matter of focus and/or intent. Either OnlineStats.jl is mainly seen as a platform for developing online algorithms, and in that case the example is fine: it shows how to implement an algorithm using the structure provided by the package. However, if the goal (as I understood it at least) is mainly to provide implementations, then I think a simple example of using the existing package functionality would be nice. This could/would also show the Plots.jl integration. Just something like the following from the docs would suffice:

o = Partition(Mean())
o2 = Partition(Extrema())

s = Series(o, o2)

fit!(s, y)

plot(s, layout = 1, xlab = "Nobs")

Other than that, lgtm

karthik commented 4 years ago

Thanks @pkofod @joshday: Can you respond to the comment above:

Either OnlineStats.jl is mainly seen as a platform for developing online algorithms, and in that case the example is fine: it shows how to implement an algorithm using the structure provided by the package. However, if the goal (as I understood it at least) is mainly to provide implementations, then I think a simple example of using the existing package functionality would be nice.

karthik commented 4 years ago

@whedon check references

karthik commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1137/141000671 may be missing for title: Julia: A fresh approach to numerical computing
- https://doi.org/10.1198/0003130042836 may be missing for title: A Tutorial on MM Algorithms

INVALID DOIs

- None
whedon commented 4 years ago

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

joshday commented 4 years ago

In response @ahwillia suggestion, I have added a section to the README about packages that use OnlineStats.

In response to @pkofod:

Either OnlineStats.jl is mainly seen as a platform for developing online algorithms, and in that case the example is fine: it shows how to implement an algorithm using the structure provided by the package. However, if the goal (as I understood it at least) is mainly to provide implementations, then I think a simple example of using the existing package functionality would be nice.

The paper is meant for a more technical audience (the "developing online algorithms" crowd) whereas the documentation is meant for general users. I'm happy to add a higher level example, but I think most people will check the documentation first.

pkofod commented 4 years ago

That makes sense. I'm fine with the current state, just wanted to be sure what the intention was. So looks good to me.

karthik commented 4 years ago

@joshday Can you please check the references and make sure it is parsing correctly?

joshday commented 4 years ago

@karthik I've fixed the reference issues!

karthik commented 4 years ago

@whedon check references

whedon commented 4 years ago 
Reference check summary:

OK DOIs

- 10.1137/141000671 is OK

MISSING DOIs

- https://doi.org/10.1007/978-1-4842-3579-9_6 may be missing for title: Spark Streaming

INVALID DOIs

- https://doi.org/10.1137/141000671 is INVALID because of 'https://doi.org/' prefix
joshday commented 4 years ago

I've fixed the invalid DOI.

The "missing" DOI I believe is in error, as I'm citing the actual software. The above DOI is for a book chapter in "Beginning Apache Spark 2", which is not what I'm citing.

karthik commented 4 years ago

If the software doesn't have a DOI, I think we can skip it but I'll tag @arfon in case he can help.

arfon commented 4 years ago

If the software doesn't have a DOI, I think we can skip it but I'll tag @arfon in case he can help.

This sounds fine. The DOI search/recommender is just using the Crossref API looking for similar titles.

karthik commented 4 years ago

@whedon generate pdf

whedon commented 4 years ago

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

karthik commented 4 years ago

LGTM

karthik commented 4 years ago

Ready to accept. Over you to @openjournals/joss-eics

kthyng commented 4 years ago

@joshday Hi! I'll take over from here.

What version number do you want associated with this publication?

Also, can you create an archive of your code at, for example, Zenodo, and report the doi back here? Please be sure that the title and author list match your paper exactly, which might requires editing the metadata.

karthik commented 4 years ago

@kthyng Yikes, sorry I forget about Zenodo and versions. doh.

joshday commented 4 years ago

@kthyng The version I'd like to use is is v1.0.3 and the DOI is 10.5281/zenodo.3659245. Let me know if there's anything else I can do to help!

A big thank you to @ahwillia and @pkofod for reviewing and @karthik for keeping this thing moving along!

kthyng commented 4 years ago

@whedon set v1.0.3 as version

whedon commented 4 years ago

OK. v1.0.3 is the version.

kthyng commented 4 years ago

@whedon set 10.5281/zenodo.3659245 as archive

whedon commented 4 years ago

OK. 10.5281/zenodo.3659245 is the archive.

kthyng commented 4 years ago

@whedon accept