UBC-MDS / software-review-2024

0 stars 0 forks source link

Group 13 Peer Review (Hexdropper) #2

Open monazhu opened 9 months ago

monazhu commented 9 months ago

Submitting Author: Mona Zhu (@monazhu) All current maintainers: (@juliaeveritt, @hchqin @joeywwwu) Package Name: Hexdropper One-Line Description of Package: A Python package designed for graphic designers, developers, and color enthusiasts that simplifies the process of obtaining hex color codes from images. Repository Link: https://github.com/UBC-MDS/hexdropper Version submitted: https://github.com/UBC-MDS/hexdropper/releases/tag/2.0.0 Editor: Tiffany Timbers Reviewer 1: Rafe Chang (@rafecchang) Reviewer 2: Weilin Han (@WeilinHan8) Reviewer 3: Rory White (@rorywhite200) Reviewer 4: Sophia Zhao Archive: TBD JOSS DOI: TBD Version accepted: TBD Date accepted (month/day/year): TBD


Code of Conduct & Commitment to Maintain Package

Description

A Python package designed for graphic designers, developers, and color enthusiasts. It simplifies the process of obtaining hex color codes from images. Given a cropped image, hexdropper quickly identifies and outputs the corresponding hex color code, streamlining design and development workflows.

Scope

Domain Specific & Community Partnerships

- [ ] Geospatial
- [ ] Education
- [ ] Pangeo

Community Partnerships

If your package is associated with an existing community please check below:

[^1]: Please fill out a pre-submission inquiry before submitting a data visualization package.

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: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.*

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.

Confirm each of the following by checking the box.

Please fill out our survey

P.S. Have feedback/comments about our review process? Leave a comment here

Editor and Review Templates

The editor template can be found here.

The review template can be found here.

rafecchang commented 9 months 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 file requirements The package meets the readme requirements below:

The README should include, from top to bottom:

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

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 whether:

Functionality

For packages also 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: 1hr


Review Comments

  1. In the example file, I think it will be easier for readers to understand if the image is placed in the section of Read image, instead of the beginning section.
  2. In the example file, it will also be nice to show the image created in the section of Export the extrated color as an image so the reader can understand how the image created will look like.
  3. In the install instruction, it is provided to place conda create --name [your_env_name] python=3.9 -y in terminal. I think it will be easier if the instruction just replace [your_env_name] with hexdropper so the user will just need to copy paste.
  4. There is no code provided to activate environment. Also the following poetry install does not run. -bash: poetry: command not found was returned.
  5. From readme, the code provided for testing is- $ pytest tests/ --cov=pycounts. I think this should be your package name instead?
WeilinHan8 commented 9 months ago

Package Review

Documentation

The package includes all the following forms of documentation:

Readme file requirements The package meets the readme requirements below:

The README should include, from top to bottom:

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

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 whether:

Functionality

For packages also 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: 1h

Review Comments

  1. [Example File] It would be great if you could include corresponding function name in headlines, the current sub-headers are not very user friendly to find the specific usage example of a specific function.
  2. [Metadata] Include user emails in the pyproject.toml
  3. [Paper.md] Please add this file
  4. [Continuous Integration] add ci-cd file
  5. [Badges] Please add other badges needed

Overall it's a really useful package and all documentation is neat and clear. Looking forward to review all the changes I proposed.

Best, Weilin

zth96 commented 9 months 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 file requirements The package meets the readme requirements below:

The README should include, from top to bottom:

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

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 whether:

Functionality

For packages also 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:


Review Comments

This is a well written package with clear documentation. I only have a few minor suggestions:

  1. Example file: I think it would be beneficial to add monospace font eg., read image into the headings to enhance readability.
  2. Example file: Some of the outputs or images are missing feedback/explanation/description/transition. By adding some brief transition lines or description of the outputs, It would make people who are new to this topic easier to understand what you are trying to demonstrate.
  3. Example file: The last function Export is missing output image. Users can render the output on their own but it would make life easier to provide the image on the website because some people like me would like to take a quick look first before running the package locally.
  4. README: The usage section is a bit too detailed and informative, it could benefit from being more concise for better readability. Try shorten the intro to one line because it is more or less the same as the main introduction and shorten the descriptions of each function.
  5. README: The installation section can be improved by using numbers, larger subheadings, or shorter description of each step to make instructions easier to read and follow. It would be easier for users to just replace $ conda create --name [your_env_name] python=3.9 -y with $ conda create --name hexdropper python=3.9 -y and let people copy, paste, and run.

Other than these readability suggestions, everything else is awesome! Great job! I look forward to reviewing the updated repo.

Best, Sophia

rorywhite200 commented 9 months 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 file requirements The package meets the readme requirements below:

The README should include, from top to bottom:

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

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 whether:

Functionality

For packages also 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

Hexdropper is an inventive package with a memorable name. It's well-documented and was enjoyable and seamless to experiment with. I put in a picture of a Giraffe and received a yellow hexcode. Excellent work!

I have come up with a few improvements that I think could elevate the package further:

  1. The function use instructions in the README.md instruct the user to run the code read_image.read_image(), however this did not work for me. I believe it should be replaced with read_image()
  2. The documentation in the README.md also refer to a variable hex_code but this is never created. I tried to run the code literally and got an error. You could rectify this by adding a line where you store the result of rgb_to_hex() as hex_code, as you did in your vignette.
  3. I encountered an error in readthedocs site, relating to fonts and matplotlib. This is important to fix as it is the first thing the user sees when they navigate to the docs.
  4. The package needs some example use cases. For instance, why would someone want to extract the predominant color in an image rather than an individual color. Please provide an example of a real-world case where this would be preferable, as this will really emphasize the value of the package in the broader ecosystem. You could add this into that section, or in the introduction.
  5. I think it would help to make it easier to get started. In the README.md, as another reviewer mentioned, you could replace replace [your_env_name] with hexdropper. You could also add in an activation statement for the conda environment and an installation line for poetry. However, I suspect that this whole section may be replaced when you create a package that can just be pip installed.