Skip to content
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

Submission: hueniversitypy (Python) #9

Open
10 of 22 tasks
reikookamoto opened this issue Mar 16, 2020 · 6 comments
Open
10 of 22 tasks

Submission: hueniversitypy (Python) #9

reikookamoto opened this issue Mar 16, 2020 · 6 comments
Assignees

Comments

@reikookamoto
Copy link

reikookamoto commented Mar 16, 2020


name: Submit Software for Review
about: Use to submit your Python package for peer review
title: ''
labels: 1/editor-checks, New Submission!
assignees: ''


Submitting Author: Reiko Okamoto (@reikookamoto), Evelyn Moorhouse (@evelynmoorhouse), Simardeep Kaur (@SimardeepKaur), Shivam Verma (@vermashivam679) (Group 16)
Package Name: hueniversitypy
One-Line Description of Package: Python package for creating visualizations in line with visual identities of Canadian universities
Repository Link: hueniversitypy
Version submitted: v1.1.0
Editor: @kvarada
Reviewer 1: @aromatic-toast
Archive: TBD
Version accepted: TBD


Description

This Python package allows users to apply university-specific themes to Altair plots. This package currently supports the official colour palettes of four institutions belonging to the U15 Group of Canadian Research Universities: University of Alberta, the University of British Columbia, McGill University, and the University of Toronto. In the future, we hope to extend this package to support the visual identities of all universities in the association.

Scope

  • Please indicate which category or categories this package falls under:
    • Data retrieval
    • Data extraction
    • Data munging
    • Data deposition
    • Reproducibility
    • Geospatial
    • Education
    • Data visualization*

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

  • Explain how the and why the package falls under these categories (briefly, 1-2 sentences):
    • The package allows users to change the theme of altair plot objects to follow certain university visual identities.
  • Who is the target audience and what are scientific applications of this package?
    • The target audience is the university staff, this package will help create visualizations that adhere to the colour palette specified by their institution's visual identity guidelines.
  • Are there other Python packages that accomplish the same thing? If so, how does yours differ?
    • Hueniversitypy is different from others as it utilizes the altair visualization package rather than other types of visualization packages. Currently, there are very few packages in the Python ecosystem that apply themes to altair plots, as described in this GitHub issue.
  • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted:

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • has an OSI approved license
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a vignette with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.

Publication options

JOSS Checks
  • The package has an obvious research application according to JOSS's definition in their submission requirements. 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: "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 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.

  • Yes I am OK with reviewers submitting requested changes as issues to my repo. Reviewers will then link to the issues in their submitted review.

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

@ttimbers
Copy link
Contributor

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all user-facing functions
  • Examples for all user-facing functions
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a setup.py file or elsewhere.

Readme requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
  • Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
  • Installation instructions
  • Any additional setup required (authentication tokens, etc)
  • Brief demonstration usage
  • Direction to more detailed documentation (e.g. your documentation files or website).
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages
  • Citation information

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

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:

  • 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).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 1


Review Comments

This is a really neat package that I definitely would use for formatting visualizations for presentations about my UBC teaching! What a great idea!

  • would be nice to have a link to the Read the Docs docs that is obvious and in english (as opposed to just the docs button, which might not be as obvious to new Python users, see the Pandas repo for example: https://github.com/pandas-dev/pandas#documentation)

  • please add how your package should be cited (formatting it as a bibTex citation would be great too!)

  • Would be neat to demo in your package how someone can add a new theme... For example, the broom package in R has a vignette called "adding-tidiers" which guides folks on adding an new broom method for different model objects.

  • It would be really neat to have a function that takes in a list of hex colours a font and creates a template function for the user... I think this could be done using templating?

  • even neater, would be for you to have a function that writes their new function to file so that they could easily submit a PR to add a new theme!

@aromatic-toast
Copy link

aromatic-toast commented Mar 21, 2020

Package Review

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README
  • Installation instructions: for the development version of package and any non-standard dependencies in README
  • Vignette(s) demonstrating major functionality that runs successfully locally
  • Function Documentation: for all user-facing functions
  • Examples for all user-facing functions
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a setup.py file or elsewhere.

Readme requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for continuous integration and test coverage, the badge for pyOpenSci peer-review once it has started (see below), a repostatus.org badge, and any other badges. If the README has many more badges, you might want to consider using a table for badges, see this example, that one and that one. Such a table should be more wide than high.
  • Short description of goals of package, with descriptive links to all vignettes (rendered, i.e. readable, cf the documentation website section) unless the package is small and there’s only one vignette repeating the README.
  • Installation instructions
  • Any additional setup required (authentication tokens, etc)
  • Brief demonstration usage
  • Direction to more detailed documentation (e.g. your documentation files or website).
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages
  • Citation information

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

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:

  • 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).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 2


Review Comments

Overall Comments

This package is very cool and super useful. It was a joy to review it. All documentation, vignette and installation instructions were clear and easy to follow. Everything worked as expected and I was able run all examples in the README with no issues. I even ran the colour themes on different datasets and plots than what was provided in the README and everything worked as expected. Fantastic job!

Additional Thoughts/suggestions:

  1. It may be useful to a user to add test data and test plots in the example for every docstring. Then, a user may run an entire example from top to bottom and see the output instantly without visiting the package README.
  2. Under the Documentation section of the reviewer template it asks for Metadata. I wasn't able to check this off because I wasn't able to find this information if it exists.
  3. As I mentioned, I tested the package out on different datasets in addition to those in the README. I noticed that if a user makes a simple barchart of a single variable the color theme will not change. This seems reasonable since aesthetically there is no need to color every single bar a different color. The README barchart changes colors because it is plotting 2 different years. I'm not sure if a note about this behaviour should be included in the docstring just to alert users to this behavior if just in case this isn't what they are expecting.
  4. Since each theme is only 5-6 colors, plots that have more than this number of categories can not be uniquely identified by color. This isn't unique to your package as I've discovered this using other palette tools in the past. But perhaps this should be explicitly noted to the user? This is probably too nitpicky and I'm not even sure this is necessary since I've seen this issue before with other palettes in ggplot and no mention was made about this.
  5. Currently it seems the build is failing on both versions of Windows but this didn't effect me.
  6. There is a tiny typo in the first line of the first paragraph in the README under hueniversitypy in the Python ecosystem. (the word alongside is misspelled)

@vermashivam679
Copy link

vermashivam679 commented Mar 26, 2020

Thank you @aromatic-toast, for providing insightful feedback to our package. We tried to incorporate all the feedback given by you, and following is our update on that:

  1. It may be useful to a user to add test data and test plots in the example for every docstring. Then, a user may run an entire example from top to bottom and see the output instantly without visiting the package README.
    • Fixed it.
  2. Under the Documentation section of the reviewer template it asks for Metadata. I wasn't able to check this off because I wasn't able to find this information if it exists.
    • We have a Contributors file in the repo that contain our names & e-mail addresses.
  3. As I mentioned, I tested the package out on different datasets in addition to those in the README. I noticed that if a user makes a simple barchart of a single variable the color theme will not change. This seems reasonable since aesthetically there is no need to color every single bar a different color. The README barchart changes colors because it is plotting 2 different years. I'm not sure if a note about this behaviour should be included in the docstring just to alert users to this behavior if just in case this isn't what they are expecting.
    • Thank you for pointing this out, but I would say a user might not find many things in our package that he is expecting. To convey this information to the user, I have included a link to the package README in the docstring.
  4. Since each theme is only 5-6 colors, plots that have more than this number of categories can not be uniquely identified by color. This isn't unique to your package as I've discovered this using other palette tools in the past. But perhaps this should be explicitly noted to the user? This is probably too nitpicky and I'm not even sure this is necessary since I've seen this issue before with other palettes in ggplot and no mention was made about this.
    • This a valid concern a user might face when dealing with categories more than what the visual identity of a university provides. For this reason, we have added a link to the university's visual identity in the docstring of every function.
  5. Currently it seems the build is failing on both versions of Windows but this didn't effect me.
    - I have noticed this too, and mainly it fails while uploading coverage report to Codecov. I don't think there is any other reason for it to fail.
  6. There is a tiny typo in the first line of the first paragraph in the README under hueniversitypy in the Python ecosystem. (the word alongside is misspelled)
    • Fixed it.

Once we have addressed feedback from all the reviewers, we will create a new release for the package and update you on that.

Regards,
Shivam Verma

@evelynmoorhouse
Copy link

evelynmoorhouse commented Mar 26, 2020

Hi @ttimbers

Thanks for the suggestions for the package! We have addressed the first two points. We moved the Read the Docs link further up in the README.md above the usage so it is obvious, and we added how the package should be cited including bibtex.

For your final three points, we agree they are all great suggestions! The ability to build a theme would allow this package to be useful globally for all universities and colleges, especially if we were to create a function to allow a user to do it. For now it is out of our scope, but is something to consider for future improvements.

Thanks!
-hueniversitypy team

@evelynmoorhouse
Copy link

Hi all,
Please see the attached link for version 1.1.1 where the changes mentioned above have been implemented.

@ttimbers
Copy link
Contributor

Nice work folks! Very cool to see these improvements to your package!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants