Skip to content

Submission: imgtoolpy (Python) #48

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

Open
11 of 22 tasks
rita-ni opened this issue Mar 17, 2020 · 5 comments
Open
11 of 22 tasks

Submission: imgtoolpy (Python) #48

rita-ni opened this issue Mar 17, 2020 · 5 comments
Assignees

Comments

@rita-ni
Copy link

rita-ni commented Mar 17, 2020

Submitting Author: Ruidan Ni (@rita-ni)
Package Name: imgtoolpy
One-Line Description of Package: A Python package that is intended to allow users to compress, sharpen and shrink an input image
Repository Link: imgtoolpy
Version submitted: v0.3.0
Editor: Varada Kolhatkar (@kvarada)
Reviewer 1: Kenneth Foo (@kfoofw)
Reviewer 2: Gaurav Sinha (@sgauravm)
Archive: TBD
Version accepted: TBD


Description

  • imgtoolpy is a Python package that is intended to allow users to compress, sharpen and shrink an input image. Our package only allows the input image to be a 3D numpy array and output the manipulated image as a 3D numpy array. It contains three functions: compress(), sharpen(), and shrink().

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 imgtoolpy package does not fall specifically into any of the categories, however it is closely related to data extraction and data visualization. It is intended to allow users to compress, sharpen and shrink an input image.

  • Who is the target audience and what are scientific applications of this package?

The target audience would be users who want to perform simple image processing without the understanding of machine learning or knowledge of using comprehensive image processing tools. This package is not intended for scientific applications.

  • Are there other Python packages that accomplish the same thing? If so, how does yours differ?

There are a few existing Python packages that perform content-aware image resizing, such as pyCAIR, and seam-carver, as well as comprehensive packages for image manipulation such as scikit-image, which could be used for filtering and transforming images. Our package is a less comprehensive image processing tool that performs three tasks: compress(), sharpen(), and shrink(), which is a more compact and easy to use package based on simple algorithms.

  • 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

@kfoofw
Copy link

kfoofw commented Mar 18, 2020

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

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

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

Estimated hours spent reviewing:

4 to 5 hours

Review Comments

General Compliments!

  • Your function code is pretty clean with easy to read documentation.
  • I really like how you state the software dependencies in your project repo, which raises awareness for the user before they install the package.
  • It was easy to install with very little dependencies required.
  • The README was very well structured and easy to understand.
  • I love the attribution and breakdown of group members by providing their Github accounts for links.
  • Your "Usage" table in your README is very minimalistic but highly effective!

Room to improve on!

  • Both compress and sharpen functions should have some example code to use to ease the user into adopting the functions in Read The Docs website.
  • It seems like the "milad_cropped.png" was compatible with sharpen and shrink, but it was not compatible with compress. A general solution might be to provide another image to work with it.
    -- Firstly, "milad_cropped.png" was a PNG file in your img folder, and it was acutally of dimensions (372, 372, 4). When I tried testing compress on "milad_cropped.png", it threw me an error since it was in (H, W, 4) instead of (H, W, 3). Perhaps this could be improved by using an image example that is suitable for all of your functions.
    -- Secondly, when I tried removing the last dimension of "milad_cropped.png" so that it is (H, W, 3), the compress function returned a black picture regardless of various b values.
  • For sharpen function, the documentation stated that it needs to take in a (H, W, 3) image input. However, it was suitable with the "milad_cropped.png" image which is (372, 372, 4). Perhaps the documentation can be revised such that it does not need to limit to (H, W, 3) images only.
  • I tried using the "mandrill.jpg" from other labs on the functions and it worked well with compress and shrink. However, there was an issue with sharpen as it only works with image data values with max value =< 1. Perhaps the function can automatically take in data and automatically normalise it by the max value so that all the values are between 0 to 1. This will reduce any rejections for the user.
  • I think the use of "milad_cropped.png" is fun as an example. However, I do wonder if you have his permission to use or distribute it since it is a personal photo of him. Given that the photo is part of your project repo located in your img folder, it does mean that his photo is publicly distributed via your project Github repo. There was no mention in your README that his permission was obtained to use/distribute his photo, and it might be viewed as a serious issue (see GDPR). To add on to that, it will be great to mention him or give him credit in your README even if he granted permission to use his photo. Alternatively, as a suggestion, you can use the mandrill photo that was used in other labs.
  • I was also wondering why your build files tested only combinations of Python 3.7, 3.8 with ubuntu operating system only instead of extending to Windows and Mac as well. Your group might have faced some limitations so this might just be a comment to note.
  • For your "Usage" table in README, I think the quoted usage might be wrong as it should be an additional function slug to use. For example, instead of imgtoolpy.compress(image, 3), it should be imgtoolpy.compress.compress(image, 3).

I'm sorry for the long feedback but I thought since you guys put in the effort to develop the package, I should also put in the effort to try it out. Thanks for the opportunity to try your package! It was fun figuring out the different functions. Please let me know if you have any questions on my feedback!

@sgauravm
Copy link

sgauravm commented Mar 21, 2020

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

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


Review Comments

The coding is neat and it is well commented which makes it easier to review and understand. The test cases has appropriate negative as well as positive test cases. There are few bugs which I noticed and also some suggestions for improvement:

  1. Include a working example in the README.md file with code and its result.You can check this repo for your reference. This will help the user understand how to use your package and can give the glipse of what each function does. It took me some time to understand how to use it.

  2. You have three separate files for each function. So the obvious command import imgtoolpy won't work for your case. The second column name of the usage section is misleading. It would be better if you mention somewhere how to import your function eg.

from imgtoolpy import compress,sharpen,shrink
  1. Function compress: Some images have more than 3 channels. I was trying one png image with 4 channels and it threw error. Please add some defensive coding for this so that the user understands what is wrong. Or you can just check for more than 3 channels and strip off the 4th channel inside your code that will make your code more generalized. If you add this write one test case for this one.

  2. Function sharpen: Although you wrote the code defensively such that it doesn't accept images having value other than 0 to 1, it makes such a powerful function very limited as most of the images have values between 0 to 255. You can just handle this by dividing your array by 255. So my suggestion would be that instead of throwing exception just divide the array by 255 and that will make your pixel intensity between 0 to 1. I tried this on my machine and it worked properly.

  3. Function sharpen: Your function takes in colored image and returns a sharpened monotone image. So there is no way to see the difference the function is making. If you could return the non sharpened monotone image along with the sharpened monotone image, that will help to make clear distinction of how well your sharpening function is performing.

  4. The docstring of all the functions doesn't have any example. It is a good practice to include example in docstring for all user facing functions.

I really found your package very interesting and it was fun doing the review. I really enjoyed playing around with the function which gave very interesting results. Hope this review can help enhance your package.

@franklu2014
Copy link

Hi @kfoofw and @sgauravm , thank you for the valuable feedback, and we have addressed some of your important comments in release 1.1.0.

More specifically, below are what have been updated:

  • Use neutral photos for demo
  • change shrink() to crop()
  • add examples to README
  • normalize input for sharpen()
  • update tests

Thank you again for your time testing our package!

@kfoofw
Copy link

kfoofw commented Mar 26, 2020

Nice new release of your package! I really like the new photos!

@Margaret8521
Copy link

Margaret8521 commented Mar 26, 2020

Thank you @kfoofw and @sgauravm for your valuable feedback. We were unable to address all your feedback due to time constraints, but here is a list of what we improved:

  • Added examples for compress and sharpen functions in the documentation.
  • Added new photos for illustration in our README.md.
  • Added defensive tests to allow users to use images of 3 channels only of the compress function
  • Improved the sharpen function so that all the values are between 0 to 1 automatically.
  • Fixed the Usage table in README.md and added usage scenarios.
  • Returned the non-sharpened monotone image along with the sharpened monotone image as shown in the README.md.

The feedback was extremely helpful, thank you again!

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