-
-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW]: rrcf: Implementation of the Robust Random Cut Forest algorithm for anomaly detection on streams #1336
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @vc1492a, it looks like you're currently assigned as the reviewer for this paper 🎉. ⭐ Important ⭐ 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:
For a list of things I can do to help you, just type:
|
|
@VivianePons My review of this software is provided below. Overview: Overall, I think the authors did a great job implementing the Robust Random Cut Forest algorithm as detailed in the original paper. Below are my comments on particular sections. When the authors strictly meet the requirements laid out in the review checklist, I have marked these items as complete. For those where the requirement is not strictly met, I have left the checklist entry empty. Either way, I have provided some comments below with actions that should be taken (in the case of unchecked entries) and actions that could be taken to improve the work further but that aren't strictly required for submission to JOSS (in the case of checked entries). I think I will give this implementation a try in a few of my projects! This project is a great implementation of the original paper and could be further improved with a little additional elbow grease. Thoroughly enjoyed reviewing this work! License: while a license is provided in the project repository (MIT), it may be beneficial to indicate the license more publicly in the readme on Github and/or in the documentation. Aa badge could be used or a section added to the readme or documentation. Functionality: I read the original paper in preparation for this review and believe the implementation here in However, when reviewing the source code I noticed a few TODO comments in Version Matching: The version number submitted to JOSS does match the latest version of the source code in Automated Tests: There are automated unit tests which I ran using Example Usage: This software does a good job of providing examples for use, including implementations of the examples from the original paper. The notebooks are also helpful, however I ran into a few issues in the notebooks that I believe should be addressed if they are to continue being included in the repository. First, some datasets are missing from the notebooks that are either 1) not available (nuclear.mat) 2) referenced in the documentation but not on the Github readme. If these datasets could be provided with links to download in the readme that could be beneficial. Second, version control should be addressed - more specifically, I not able to run some of the notebook code with earlier versions of Matplotlib and Pandas but was later to successfully after an update. It would be helpful to new users of the software to have a list of required versions for packages needed to run the examples in the notebooks. Lastly, in the documentation on the website the file Installation Instructions: Community Guidelines: References: |
@VivianePons My review of this software is provided below. Overview: I think the authors did a great job implementing the Robust Random Cut Forest algorithm as a Python package. I am also looking forward to implementing this in a few of my projects. Following @vc1492a's thorough review, I have little to add, so my review will rather emphasize the points already addressed which I think are most relevant to the authors of this project. The code is well commented and described in sufficient detail, which supports future development. License: +1 to adding a badge to the heading. Functionality: Nothing to add. Version Matching: +1 to tagging releases uploaded to GitHub. Automated Tests: Nothing to add. Example Usage: +1 to cleaning up the notebooks if they are to remain. Installation Instructions: +1 to putting installation instructions towards the top of document. Community Guidelines: Nothing to add. References: Nothing to add. |
Hi @vc1492a and @JustinShenk, thanks for the excellent reviews! I will get to work on these issues as soon as possible. |
@whedon set 0.3 as version |
I'm sorry @mdbartos, I'm afraid I can't do that. That's something only editors are allowed to do. |
Greetings, @vc1492a and @JustinShenk: Thank you again for the extremely relevant and constructive feedback. It would not be an exaggeration to say that this is one of the most helpful reviews I have received on an academic paper :) A point-by-point summary of changes is listed below: License
Functionality
Version matching
Automated tests
Example usage
Installation instructions
Community guidelines
References
Please let me know if you have any other suggestions. Your time, effort and expertise is very much appreciated. Thanks again! |
@VivianePons : Would it be possible to change the version to 0.3? I tried to do this using whedon but was denied. Also, would it be possible to change the description on JOSS to:
Thank you! |
@mdbartos thanks for the reply and for integrating my pull request! @VivianePons I have reviewed the changed @mdbartos put in place - they look great and I've updated the review checklist to reflect the latest status. Please let me know if there's anything else I can take care of as part of the review process. Thanks! |
@whedon set 0.3 as version |
OK. 0.3 is the version. |
@whedon generate pdf |
|
@mdbartos I have updated the version but I'm not sure which description you're referring to |
@JustinShenk Have you looked at the most recent changes? Do you give your green light for accepting the paper? |
Hi @VivianePons, I was just referring to the description on the JOSS website: https://joss.theoj.org/papers/f8c83c0b01a984d0dbf934939b53c96d I wanted to add "rrcf is a ..." to the beginning of the description to make the description more in line with the style of other submissions. It's not a big issue though. |
@arfon do you know where this description is coming from: https://joss.theoj.org/papers/f8c83c0b01a984d0dbf934939b53c96d ? |
Yes, it's what the author included in the form when they originally submitted. Once accepted, this text is never public so we can mostly ignore it. |
@whedon generate pdf |
|
ping @JustinShenk |
@VivianePons Yes, I've reviewed the recent changes from @mdbartos and updated the checklist. Looks good to me! |
@whedon check references |
|
|
This paper is good to go! :) @mdbartos could you create an online archive (for example on Zenodo) and give the reference? |
Hi @VivianePons, I created an archive with Zenodo using their Github integration service: Unfortunately, to get the integration to work I had to update the release to 0.3.1 Would it be possible to update the version to 0.3.1 for consistency? Thanks again, |
@whedon set 0.3.1 as version |
OK. 0.3.1 is the version. |
Can you update the Zenodo metadata so that the title matches the paper title? Your name and Sara Troutman's name are also a bit different in both (missing the middle name letter) and the order is different. Thanks Viviane |
Ok, it is done. Thanks, |
@whedon set 10.5281/zenodo.2613881 as archive |
OK. 10.5281/zenodo.2613881 is the archive. |
This paper is now good to go @openjournals/joss-eics ! Thank you @mdbartos for the paper and @JustinShenk and @vc1492a for the reviews! :) :) |
And thanks @VivianePons for editing |
@whedon accept |
|
|
Check final proof 👉 openjournals/joss-papers#587 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#587, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? notify your editorial technical team... |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: 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:
|
Submitting author: @mdbartos (Matthew Bartos)
Repository: https://github.com/kLabUM/rrcf
Version: 0.3.1
Editor: @VivianePons
Reviewer: @vc1492a, @JustinShenk
Archive: 10.5281/zenodo.2613881
Status
Status badge code:
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
@vc1492a & @JustinShenk, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @VivianePons know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @vc1492a
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?Review checklist for @JustinShenk
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
paper.md
file include a list of authors with their affiliations?The text was updated successfully, but these errors were encountered: