UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 1 - magmaviz (Python) #44

Open miyer26 opened 2 years ago

miyer26 commented 2 years ago

Submitting Author: Mukund Iyer (@miyer26)
Package Name: magmaviz One-Line Description of Package: Makes EDA easy by providing functions for four common types of plots with the magma color scheme. Repository Link: https://github.com/UBC-MDS/magmaviz Version submitted: 0.3.9
Editors: Mohammed Abdul Moid (@iamMoid), Irene Yang (@shyan0903), Rubén De la Garza Macías (@ruben1dlg), Mukund Iyer (@miyer26)

Reviewer 1: Yair Guterman @gutermanyair Reviewer 2: Giang Nguyen @gn385x Reviewer 3: Daniel King @danfke


Description

This package aims to make Exploratory Data Analysis easy by providing python plot functions based on the Altair library to plot four common types of plots with the magma color scheme. To maximize interpretability, the plots have defined color schemes (discrete, diverging, sequential) based on the kind of data they show and additional inputs for customizability.

Scope

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

This package has functions with the sole objective of creating plots promptly and easily based on the 'Altair' package using the magma color scheme.

This package is designed to be used by data scientists who are exploring data or developing a visual dashboard. The package is versatile and can be used for most data science projects.

Similar packages to ours include deneb, which uses Altair, and spartan-viz, which uses Matplotlib. These also streamline the EDA process but do not use the Magma theme and functionalities that this package offers.

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

gutermanyair 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

You have an amazing package but here are a few comments for you guys! <3

  1. The codecov badge does not show the percentage of coverage, even though when I click it I can see the percent!

  2. There is not much detail in comparing other similar packages in the readme, I am aware that there must be many EDA packages out there!

  3. I see that your plots only display in one color scheme, if people want to use the EDA plots in their report maybe adding in an option for a choice of color scheme would be very nice!

  4. If I could add more stuff to your package I would maybe want to include violin plots and heat maps and then your package, in my opinion, would cover everything needed for an EDA report

  5. 82 percent is some nice coverage but as a user of your package, I would love to see that number be over 90, maybe include some more tests to make sure all your code is being tested.

I love you all especially Moid , keep up the amazing work you all!

danfke 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 hour


Review Comments

  1. After installing using the installation instructions and then duplicating the boxplot function example from the Usage section of the README, I get an error saying that facet is an unexpected keyword argument. This is why I kept functionality unchecked.

  2. I have a feeling that the installation is for an old version of the package which is probably why the facet argument didn't work in the boxplot function. The installation instructions do refer to test PyPi.

  3. Code coverage is good but getting over 90% would be great!

  4. I can't seem to find the author's emails which is why I left metadata unchecked. As far as I could find by looking through the readme, license, and contributing guidelines there is single email for the whole team instead.

  5. In the example usage of the hisogram function, I'm not really sure what's going on, it seems like there are overlapping bars? For example it seems like there are black bars in the foreground and lighter bars in the background at some values of horsepower. Is this a possible functionality glitch? Maybe the bins are too small? Having a bin argument in the histogram function I think would be useful to users!

Overall beautiful looking package with well-written documentation. Great job!

gn385x 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

An amazing package guys! A few suggestions for improvement:

  1. The README is descriptive and informative. Two suggestions to improve the file:
  1. All functions are very well documented and easy to follow. You could give users more flexibility by adding an additional function at the beginning of your package to enable importing any data sets that users are interested in (rather than using data from vega_datasets only).
  2. In the future, you might want to consider allowing users to customize plots a bit more. For example, you might be able to let the users choose the color of the figures they want.
  3. Echoing two reviewers above, it would be nice to increase the code coverage to be over 90%. One suggestion is to add more (sophisticated) tests so that all your functions and edge cases are tested.