UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 27: covizpy (Python) #51

Open lirnish opened 2 years ago

lirnish commented 2 years ago

Submitting Author: Rohit Rawat (@rrrohit1), Rong Li (@lirnish), Thomas Siu (@thomassiu), Ting Zhe Yan (@ytz) Package Name: covizpy One-Line Description of Package: Python package for easily visualizing COVID-19 statistics Repository Link: https://github.com/UBC-MDS/covizpy Version submitted: 1.0.23 Editor: Rohit Rawat (@rrrohit1), Rong Li (@lirnish), Thomas Siu (@thomassiu), Ting Zhe Yan (@ytz) Reviewer 1: Sukhleen Kaur (@sukhleen999) Reviewer 2: Philson Chan (@PhilsChan) Reviewer 3: Kingslin Lv (@Kingslin0810)


Description

Scope

* Please fill out a pre-submission inquiry before submitting a data visualization package. For more info, see notes on categories of our guidebook.

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

Kingslin0810 commented 2 years ago

Package Review

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

Documentation

The package includes all the following forms of documentation:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

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:

Functionality

For packages co-submitting to JOSS

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:

Final approval (post-review)

Estimated hours spent reviewing:

2.5 hours

Review Comments

Congratulations group 27 and well done on your covizpy package! It's a well-organized and user-friendly package for anyone with basic python programming knowledge to quickly understand the covid-19 data summaries. Overall, I think your project is well designed. Some highlights are the package repo is clearly structured, mostly everything required being included, easy to follow, and the written documentation being very well organized which is simple to follow.

I have a few recommendations for improvement:

  1. Might consider adding exception handing for dates prior to “2020-01-01” when the covid data was not generated for the function get_data.
  2. The input argument location in the function plot_summary (var input), plot_spec, and plot_metric are optional but they are built based on whatever input is entered in the function get_data. Maybe this needs to be clearly documented in the docstring, and so we don't run into below unclear presentation. screenshot1
  3. The README file looks good, but I think it would be better to explicitly list a direct weblink to your documentation as opposed to just include the docs badge.
  4. A nitpicking thing in your README should the function get_data be always the first to run prior to others? Like the function is listed in the third under Features.
  5. Your example doesn’t have output plots exhibited which makes audiences hard to understand your functions. I haven’t tested yet but alt.renderers.enable('mimetype') might save a backup image of each Altair plot as a PNG so that we can see the plots in output.
  6. Your documentation page has two repetitive logos in the front page which are distracting to your contents for presentation. I suggest you keep one logo.

Other than above minor suggestions, I feel that the code is in a good state; again, great work team, I really enjoyed reading through this. Thank you.

PhilsChan commented 2 years ago

Package Review

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

Documentation

The package includes all the following forms of documentation:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

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:

Functionality

For packages co-submitting to JOSS

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:

Final approval (post-review)

Estimated hours spent reviewing: 1.5 hours


Review Comments

The package looks very neat and cool in general. The documentations are clear and easy to follow, in which users with little python programming skills could use this handy visualization tool without any barrier. The pipeline for data retreival and visualization are seamless, and the charts could present the data clearly.

Here are some recommendations and suggestions that you might work on for the future:

  1. The function interface could be more unified. Functions plot_spec and plot_summary both have df as their primary input, while plot_metric does not. This might confused the users a bit, and I suggest that all the plotting functions could consider using similar function interface and keyword arguments so that the user experience would be smoother.
  2. Similar to the issue above, the behaviour of some default arguments are different among the functions. For example, the default behaviour of date_from and date_to for plot_metric are setting it to two actual date, while the other functions set the range to 7 days prior to the current date.
  3. The functions could be more flexible with different input format. For instance, as the output of get_data is a pandas dataframe, where its date column is in datatype of pandas.Timestamp, you might consider support such datatype as input for arguments like date_from and date_to. I believe it could enhance the usability of the package when users want to play around with the data set.
  4. For the function plot_summary, the argument fun only accept string object. However as a user, I don't know what could I input for the parameter apart from "sum". I suggest that you might put a list of possible values for this particular argument in the documentation. It is also suggested that a function or lambda could be passed to the argument so that users could defined their own aggregate functions.
  5. The README file shall include link to the readthedocs documentation.
  6. In the documentation, plot_metric do not have an example section. I think adding the section could make the whole package more unified and easier for users to follow.

Apart from the minor concerns stated above, the package in general is already so comprehensive and ready-to-use. I really appreciate that you could make the data visualization much easier for non-tech people with this package. Thank you!

sukhleen999 commented 2 years ago

Package Review

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

Documentation

The package includes all the following forms of documentation:

Readme requirements The package meets the readme requirements below:

The README should include, from top to bottom:

Usability

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:

Functionality

For packages co-submitting to JOSS

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:

Final approval (post-review)

Estimated hours spent reviewing: 2 hours


Review Comments

Great job team! The package seems extremely helpful for beginners who wish to visualize the trend of Covid-19 cases and draw meaningful conclusions. I found the installation and the functions extremely smooth to work with. The README is also very well structured.

For further improvement, I suggest the following changes:

In summary, I really liked the inspiration of the package and would definitely be using it to keep abreast of the Covid-19 cases.