Open jamesh4 opened 4 years ago
The package includes all the following forms of documentation:
URL
, BugReports
and Maintainer
(which may be autogenerated via Authors@R
).Estimated hours spent reviewing: 2 hours
Overall a very well-built package!
devtools::check()
and it successfully ran after I updated some dependencies. A list of dependencies may be helpful if added to README.md
file.
In the README.md
file, in the usage section, there exists a typo in the cluster_summary
function. I believe it was meant to be clustersummary
.
In your predict.R
file the documentation mentions an extra distance_metric
argument which is not part of the current implementation. Consider updating the documentation for this function.
The second and the third tests in test-predict.R
-which are testing points with the same coordinates being assigned to the same centroid - are the same. Consider removing one of them.
Tests can be added to test-elbow.R
for checking the input's format. The coverage will increase after testing the 4 if branches.
In clustersummary.R
the example in the documentation has a typo, it calls cluster_summary
instead of clustersummary
.
Other than that, the package is well implemented, the documentation is well written and the tests covered the major functionality of the package.
Thanks again. I truly found it interesting and really helpful to go through this work. Let me know if you have any questions.
Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide
The package includes all the following forms of documentation:
URL
, BugReports
and Maintainer
(which may be autogenerated via Authors@R
).For packages co-submitting to JOSS
- [ ] The package has an obvious research application according to JOSS's definition
The package contains a
paper.md
matching JOSS's requirements with:
- [ ] A short summary describing the high-level functionality of the software
- [ ] Authors: A list of authors with their affiliations
- [ ] A statement of need clearly stating problems the software is designed to solve and its target audience.
- [ ] References: with DOIs for all those that have one (e.g. papers, datasets, software).
Estimated hours spent reviewing:
2 hours
These results are quite impressive given how complex these functions are.
Readme of the package looks perfect. It was quite helpful in giving an overview of what is happening in each function. Although I noticed a small typo. You should consider changing the fucntion “cluster_summary” to “clustersummary” because that is the actual function name.
This package looks great and is really inspiring for someone whi wants to have a deep understanding of a particlar algorithm to go ahead and try building it from scratch. I really enjoyed going through all the functions and docuemenation. Keep up the good work!
Hi @EitharAlfatih and @SimardeepKaur , we're happy to be able to tell you that we've implemented almost all of your feedback in our latest package release 1.1.0.
tidyverse
as our only dependency in the package README. clustersummary
typos that were in the README usage section and the function's documentation.predict
function, and also removed the description of the non-existent distance_metric
argument in the predict
function documentation. elbow
function to check for correct inputs. We also added several other tests for other functions to increase the overall code coverage. Thank you very much for the feedback, it has definitely improved the quality of our package!
name: KmeansR about: K-means package for R title: K-means implementation from scratch labels: 1/editor-checks, New Submission! assignees: Eithar Elbasheer (@EitharAlfatih), Simardeep Kaur (@SimardeepKaur)
Submitting Author: Rob Blumberg (@RobBlumberg ), Sreejith Munthikodu (@sreejithmunthikodu ), Saurav Chowdhury (@saurav193 ), James Huang (@jamesh4 ) Package Name: KmeansR One-Line Description of Package: K-means implementation from scratch Repository Link: https://github.com/UBC-MDS/KmeansR/tree/master Version submitted: https://github.com/UBC-MDS/KmeansR/tree/1.0.0 Editor: TBD
Reviewer 1: Eithar Elbasheer (@EitharAlfatih) Reviewer 2: Simardeep Kaur (@SimardeepKaur) Archive: TBD
Version accepted: TBD
Description
Scope
* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see this section of our guidebook. * The data exploration category was added after consultation with @kvarada since the other categories don't accurately describe this package.
This package implements the k-means algorithm, a data mining and clustering technique used to uncover relationships in unlabelled data.
This package was created to provide a deeper understanding of the k-means clustering algorithm. Thus, this package is targeted to anyone interested in diving into the implementation of this clustering technique.
There is a built-in R function called KMeans. This package is not meant to add to the existing ecosystem; it is rather intended to deepen fundamental understanding of these algorithms.
@tag
the editor you contacted:We spoke to @kvarada and she approved the addition of the data exploration category.
Technical checks
For details about the rOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:
Publication options
JOSS Checks
- [ ] The package has an **obvious research application** according to JOSS's definition in their [submission requirements](https://joss.readthedocs.io/en/latest/submitting.html#submission-requirements). Be aware that completing the rOpenSci review process **does not** guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS. - [ ] The package is not a "minor utility" as defined by JOSS's [submission requirements](https://joss.readthedocs.io/en/latest/submitting.html#submission-requirements): "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." rOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria. - [ ] The package contains a `paper.md` matching [JOSS's requirements](https://joss.readthedocs.io/en/latest/submitting.html#what-should-my-paper-contain) with a high-level description in the package root or in `inst/`. - [ ] The package is deposited in a long-term repository with the DOI: *Note: Do not submit your package separately to JOSS*Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?
This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.
Code of conduct
P.S. Have feedback/comments about our review process? Leave a comment here
Editor and Review Templates
Editor and review templates can be found here