UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 09: ImgHelp #17

Open jasmineortega opened 2 years ago

jasmineortega commented 2 years ago

Submitting Author: Sufang Tan @Kendy-Tan, Jasmine Ortega @JasmineOrtega, Ho Kwan Lio @stevenlio88, Maeve Shi @MaeveShi Package Name: ImgHelp One-Line Description of Package: mgHelp is a simple image processing Python package. Repository Link: https://github.com/UBC-MDS/ImgHelp Version submitted:
Editor: TBD
Reviewers: Paniz Fazlali @paradise1260, Melisa Maidana @mmaidana24318), Lianna Hovhannisyan @liannah

Archive: TBD
Version accepted: TBD


Description

imghelp is a simple Python package to help users crop, rotate, compress, or change the color scale of a given image. It contains four functions: Crop(), ImgRotate(), ColorConv() and ImgCompress() and is designed to be a beginner-friendly image processing tool.

Scope

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

Technical checks

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

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

paradise1260 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 h

Review Comments

I really enjoyed reviewing your package. It has useful functionalities. Here are my comments after reviewing the package:

  1. ColorConv(image, 'purple') does not throw a useful error. I realized the colours should be 'red', 'blue', 'green' or 'gray'. However, the function does not raise a developer-defined error for a colour other than those. You can probably check if the entered colour is among the acceptable colours.
  2. ImgCompress(image, method = 'Resize', level=1) throws an error saying 'index 1600 is out of bounds for axis 1 with size 1600'. I used a jpg image of size 2500x1500. You probably can address that issue in the function definition.
  3. I found it interesting that Crop() function prints out a message saying that the conversion was successful.
  4. imgrotate() rotates the image based on the desired degree going through if statements. The statements are very similar. Maybe the function can be recursive, so the function calls itself.
  5. The example usages in ReadTheDocs were informative and straightforward.
  6. You can add a docs badge to your README.md. If you go to ReadTheDocs website where you imported your repository, you can copy the badge for markdown and paste it in README.md.
  7. The functions are well-documented.
  8. You can probably add a very brief example of each function in README.md.

Finally, nice job team!

mmaidana24318 commented 2 years ago

Package Review

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. The example of usage in ReadTheDocs is missing a use case of the Crop function. Beyond that, the notebook is very easy to follow.
  2. I appreciate that the Crop, ImgRotate and the ImgCompress functions throw meaningful error messages when providing a parameter value other than the ones expected. Hopefully you can easily replicate that for ColorConv.
  3. There is a typo in the docstring of ColorConv. I accessed help(ColorConv) because the function failed after I try to run ColorConv(img, 'yellow') so I wanted to check the permitted colors. It states "rad" instead of "red".
  4. The ImgCompress function with the resize method only works with square images. I couldn't make it work with rectangular  photo (faced an out of bounds index error).
  5. You may want to invest some time adding more tests cases. Currently, the 4 functions are only testing squared images. A more robust testing should have caughtthe error on ImgCompress.

The package seems very helpful and it's definitely friendly for a beginner user. The installation is smooth and the documentation is clear. Great job!

liannah 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. Very nice job, I really liked how the package was presented and well documented. Though, I could not find where I can contact the team if I have an issue except development one. For instance, I want to discuss licensing or attribution. I think it will be nice to have an email present for contact. I was hoping to see it in the conduct.md or contributing.md, but I could not spot it.
  2. The file contributors.md feels additional, as it has only the names of contributors without any additional contact information. Maybe, you can unite the contributing and contributors files into one contributing file, if there is no other reason why they should be separate.
  3. The installation of the package was smooth, and I enjoyed playing around the pictures. Though, I noticed that only square images worked. I would suggest adding the restriction note, that the images need to be squared or preprocessing step in functions that make the picture squared before applying rotations, or cropping.
  4. Thank you for including the brief introduction to each function in readme. I would suggest making the start of the sentence more robust, meaning getting rid of this function statement, and instead separate with semi colon function name: takes an image. Also, it will be nice to have short examples in readme present as well, or the provided link for in-depth look to go straight to the example page in the documentation website.
  5. The code is well documented, and tests seem sufficient. I have noticed that there is a typo in ColorConv docstring, misspelled red with rad.
  6. I will also suggest having the batches besides each other not in vertical format in readme document, as it looks like a lot of space is left blank.

Overall, very good package and amazing job. I enjoyed playing around the pictures, thank you for creating the package and making it open source.