Open voremargot opened 2 years ago
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:
setup.py
file or elsewhere.Readme requirements The package meets the readme requirements below:
The README should include, from top to bottom:
Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.
The package contains a paper.md
matching JOSS's requirements with:
Estimated hours spent reviewing: 45 minutes
Nice work! The project is well organized and the documentation is helpful and easy to understand. Here are my suggestions:
Since the package was built on NumPy and Matplotlib, they need to be referred to in the citation section in README.
Although the coverage of test cases has reached 100%, there are few edge cases.
Some simple examples can be added to the Usage part so that users don't need to read the entire doc to understand your package.
Re-run example.ipynb, so the cell index starts from 1.
In the ci-cd.yml, you only test ubuntu. It's necessary to test Window and macOS to ensure your package is compatible with other systems.
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:
setup.py
file or elsewhere.
Readme requirements
The package meets the readme requirements below:Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
paper.md
matching JOSS's requirements with:This project is very relevant to our worn Statistics, and I thoroughly enjoyed reviewing it. The repository is well structured and had clear instructions on how to install and use the functionality. All the functions are working as per the documentation and test cases cover a broad range of scenarios.
I do have a few comments for your consideration:
replacement
as an optional argument. This will allow the user to specify if bootstrapping is to be done with or without replacement.Overall, I think that the work done is impressive in terms of functionality as well as documentation. I wish you the best as you develop it further.
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:
setup.py
file or elsewhere.Readme requirements The package meets the readme requirements below:
The README should include, from top to bottom:
Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.
The package contains a paper.md
matching JOSS's requirements with:
Estimated hours spent reviewing: 1.5 hrs
Great work with this package! The functionalities are extremely useful and I especially appreciate the package catering to DSCI 552. Below I have mentioned some points that you can improve on and make the package more user friendly:
In the Readme, it would nice to write information more in bullet points and highlight key things you would definitely want the users to take away. I am especially talking about the "Package context within the Python ecosystem" section, I think it would make this section more clear if each bullet covers a specific point.
Following up on the "Package context within the Python ecosystem" section, I feel there needs to be more elaboration on how this package compares to other packages out there and what is different about this package. Right now you mention what strapvispy builds on and also briefly mention scikit learn, but you don't exactly talk about how those packages do boostrapping and how is strapvizpy able to streamline the process. Also it would good to elaborate more on some of the boostrapping jargon.
It is a bit hard to understand your source code and I would recommend that more comments be added to explain what some of the branches do and also to separate the branches with spaces. It would be nice to have a small phrase describing what exception handling you are doing.
In your ReadMe or documentation it would also be nice to mention if users need a certain python version to install your package.
I also noticed some small inconsistencies in your package, like for example your package is named "strapvizpy" and this is how your users would install and import it, but sometimes in your Readme and documentation you instead say "StrapvizPy". I think in general it would be good to stick with "strapvizpy" making it less confusing. Additionally, in your test code while some error checks are well documented through comments, others lack them.
All in all, great job! It very clear that a lot of thought and effort has been put into this package. It is very detailed and it would be interesting to see if this will be used by the next cohort in DSCI 552!
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:
setup.py
file or elsewhere.Readme requirements The package meets the readme requirements below:
The README should include, from top to bottom:
Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole. Package structure should follow general community best-practices. In general please consider:
Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.
The package contains a paper.md
matching JOSS's requirements with:
Estimated hours spent reviewing: 1
Wonderful little package.
Excellent documentation.
Can remove the "pip install" bit. Very common issue in this block's packages.
Usage syntax is a little funny what with needing the prefix "bootstrap.calculate_boot_stats()"
I saw another comment mention lack usage examples. Not sure if it was submitted before the end of the appropriate milestone, but I thought the usage examples in ReadTheDocs was more than enough to get started with the package.
Similar to my feedback in the R version of this package, I find the red v-line for the estimate against the blue histogram a little jarring on the eyes.
I really appreciate human-readable table titles! I think maybe it would be cool to have an argument in the tabulate_stats() function that would let you pick human readable labels vs coding-friendly ones (with underscores and small letters)? I realize that the package's purpose is primarily to whip out publish-ready tables and visualizations easily, but seems like this would be a useful and simple feature.
I love the division into "Bootstap" and "Display" sections. So nice to have low-key suggestions for workflow within documentation.
Wonderful job friends, you should be proud! 😊🌸
Submitting Author: Julien Gordon (@BooleanJulien), Gautham Pughazhendhi (@gauthampughazhendhi), Zack Tang( @zackt113), and Margot Vore (@voremargot) Package Name: StrapvizPy One-Line Description of Package: Used for bootstrapping visualizations Repository Link: https://github.com/UBC-MDS/strapvizpy Version submitted: v0.2.4 Reviewer 1: Rada Rudyak (@Radascript) Reviewer 2: Arushi Ahuja (@Arushi282) Reviewer 3: Zheren Xu (@ZherenXu) Reviewer 4: Anupriya Srivastava (@Anupriya-Sri)
Description
The purpose of this package is to simplify and automate the process of creating simple bootstrap distributions of numerical samples. The package has a module which intakes a sample and relevant parameters such as the desired confidence bounds and number of simulations. The module will perform the simulation statistics to generate the bootstrap distribution and relevant statistics such as the sample mean and bootstrapped confidence interval. The package also has a module for visualization of the bootstrapped confidence interval, and for creating a professional publication-ready table of the relevant statistics.
Scope
Please indicate which category or categories this package falls under:
Explain how the and why the package falls under these categories (briefly, 1-2 sentences):
Who is the target audience and what are scientific applications of this package?
Are there other Python packages that accomplish the same thing? If so, how does yours differ?
Technical checks
For details about the pyOpenSci 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][JossSubmissionRequirements]. Be aware that completing the pyOpenSci 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][JossSubmissionRequirements]: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria. - [ ] The package contains a `paper.md` matching [JOSS's requirements][JossPaperRequirements] 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