UBC-MDS / software-review-2024

0 stars 0 forks source link

image_modifier - Group 8 #13

Open karan-khubdikar opened 5 months ago

karan-khubdikar commented 5 months ago

Submitting Author: Karan Khubdikar (@karan-khubdikar) All current maintainers: (@lichunubc, @celestezhao, @hema2022ubc ) Package Name: image_modifier One-Line Description of Package: Function to modify images (rotating/slicing/adding frame/changing channels) Repository Link: https://github.com/UBC-MDS/image_modifier Version submitted: 2.0.0 Editor: @ttimbers
Reviewer 1: Iris Luo (@iris0614) Reviewer 2: Beth Ou Yang (@beth-ouyang) Reviewer 3: Prabhjit Thind (@Prabh95) Reviewer 4: Oak Chongfeungprinya () Archive: TBD JOSS DOI: TBD Version accepted: TBD Date accepted (month/day/year): TBD


Code of Conduct & Commitment to Maintain Package

Description

ImageModifier is a Python package dedicated to providing an intuitive and efficient way to manipulate images. This package focuses on core image processing functions, allowing users to easily modify images through operations like rotating, slicing, adding frames, and adjusting RGB channels. The primary goal is to offer a straightforward way for basic yet powerful image transformations, making it a useful tool for image processing.

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.

beth-ouyang commented 5 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

iris0614 commented 5 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  

Prabh95 commented 5 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.5 hrs


Review Comments

I like the whole idea of this package which is similar to OpenCV and Pillow, for providing functionalities around image processing.

Positives:

  1. Installation instructions were pretty concise and easy to follow
  2. Docstring details and Error handling is on point. High test coverage.
  3. I like the easy readibility and narration of the example notebook

Improvements:

  1. Rename Example to Vignette: Your package has the Vignette notebook but it is named as Example.ipynb. Example and Vignette documentations have their own distinct purposes. Example notebook is a more concise and code-centric document that showcases specific use cases and functionalities of a package. A vignette is a more extensive and narrative-style document that explains the concepts and features of a package in a tutorial-like manner. Your example notebook is actually a Vignette since its a narration and I would suggest to rename it and optionally create a separate example notebook if required.
  2. Pytest images: The test images generated during pytest run lack details and supplemental information for someone new testing it. How do I know if the images are correctly generated or not? Could also do away with those images.
  3. Avoid printing the whole numpy array in Documentation: In the example notebook, instead of printing the whole numpy array could just print the variable datatype. The documentation would look cleaner by doing that as it is unnecessarily increasing the document length and extra scroll. Could use isinstance method, isinstance(my_variable, np.ndarray) to confirm for an np array.
sivakornchong commented 5 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.5 hr


Review Comments

Great package so far! The installation instruction is easy to follow and I have completed them accordingly. The Example.ipynb works as per intended.

Three pieces of comments for possible improvements here:

  1. The enviroment file may no longer be necessary since the virtual environment is already being managed in the poetry file. The whole thing could run on my base environment. In that case, to run the jupyter lab, please remember to add jupyter-lab into the poetry.
  2. I tried running pytest on my computer. It passed everything besides this function at def test_slice_image_edge_cases(). The error is C:\ProgramData\miniconda3\envs\image_modifier\lib\tkinter__init__.py:2270: TclError. I tried this on the image_classifier environment and there is still this error...
  3. Branches management on github repository: I think it would be great to remove some unnecessary branches on the repository

Overall, great work! The documentations are excellent and I really enjoy the process.