Submission: imageprocessor (Python)

Submitting Author:

Joshua Lim @jiajie0225
Mo Garoub @mgaroub
Rui Wang @wang-rui
Aishwarya Gopal @AishwaryaGopal12

Package Name: imageprocessor One-Line Description of Package: A Python package that aims to make the image processing task easy and intuitive. Repository Link: imageprocessor Version submitted: 0.3.0 Editor: Tiffany Timbers (@ttimbers) Reviewer 1: TBD
Reviewer 2: TBD
Archive: TBD
Version accepted: TBD

Description

Images form a significant part of the data in today's world. Whether you want to enhance your poorly-lit profile picture or analyze satellite images, filters are your best friends. In scientific image processing, at most times, the images cannot be used directly and need to be processed to extract information from them. ImageProcessor aims to make the image processing task fast, easy, and intuitive. With ImageProcessor, users can rotate an input image, converting an input image to grayscale, flipping an input image in either a horizontal or vertical direction, and add padding (frame) to an input image.

Scope

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

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

Explain how the and why the package falls under these categories (briefly, 1-2 sentences):
- The imageprocessor package does not fall specifically to any of the categories, however, it is closely related to data extraction and data visualization as its intention is to allow users to rotate, flip, pad, and apply grayscale to an input image.
Who is the target audience and what are scientific applications of this package?
- The target audience would be simply users that want to apply image processing tasks on an input image without going through the hassle of writing code or downloading a third partyy software. It is most useful to users that do not have any understanding of machine learning or knowledge of using comprehensive image processing tools.
Are there other Python packages that accomplish the same thing? If so, how does yours differ?
- Yes, there are plenty of packages with image and advanced image processing functionalities. One of the most comprehensive image processing libraries in Python is scikit-image. Our package is a less comprehensive image processing tool that performs four tasks: rotate(), flipping(), padding(), and greyscale(). We are looking to simplify some of the image processing features from the comprehensive packages.
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:

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

Publication options

[ ] Do you wish to automatically submit to the Journal of Open Source Software? If so:

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

[x] 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

[x] I agree to abide by pyOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

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

Assigning @spentelow & @rtaph as reviewers.

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

[X] 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).
- Note that I am in the same UBC MDS cohort as the authors

Documentation

The package includes all the following forms of documentation:

[X] A statement of need clearly stating problems the software is designed to solve and its target audience in README
[X] Installation instructions: for the development version of package and any non-standard dependencies in README
[ ] Vignette(s) demonstrating major functionality that runs successfully locally
- Usage descriptions are present for all functions in readme but it would be helpful to have some more context (perhaps screenshots of the output of each function on a sample image?)
[X] Function Documentation: for all user-facing functions
[ ] Examples for all user-facing functions
[X] Community guidelines including contribution guidelines in the README or CONTRIBUTING.
[X] 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:

[X] Package has a README.md file in the root directory.

The README should include, from top to bottom:

[X] The package name
[X] Badges for continuous integration and test coverage, 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. Such a table should be more wide than high. (Note that the badge for pyOpenSci peer-review will be provided upon acceptance.)
[X] 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.
[X] Installation instructions
[ ] Any additional setup required (authentication tokens, etc)
- N/A
[X] Brief demonstration usage
[X] Direction to more detailed documentation (e.g. your documentation files or website).
[X] If applicable, how the package compares to other similar packages and/or how it relates to other packages
[ ] Citation information

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:

[X] The documentation is easy to find and understand
[X] The need for the package is clear
[X] All functions have documentation and associated examples for use

Functionality

[ ] Installation: Installation succeeds as documented.
- Package successfully installed with poetry install, but the given installation commands do not work
[ ] Functionality: Any functional claims of the software been confirmed.
- All functions except 'padding' produce the expected output image files. 'padding' function always adds a 1 px border instead of the user specified padding amount (see 'Review Comments' section below for additional notes)
[X] Performance: Any performance claims of the software been confirmed.
[X] Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
[X] Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
[X] Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.

For packages co-submitting to JOSS

[ ] The package has an obvious research application according to JOSS's definition in their submission requirements.

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:

3.5

Review Comments

Hi Joshua, Mo , Rui, and Aishwarya,

Thanks for putting this package together. I had fun playing around with some pictures using your functions! The way the functions work is very intuitive and they are definitely a time saver over playing with a bunch of matplotlib functions and transformations.

I've made note below of a few things that I think would improve the overall package. Most things are working well but I have noted a couple of items I had issues getting to work. Other than that, my comments are just on 'polishing' items (documentation etc.).

The installation instructions as documented did not work for me. Perhaps the following command could be used instead:
- pip install --index-url https://test.pypi.org/simple/ --extra-index-url https://pypi.org/simple imageprocessor
The user specified padding size in the 'padding' function does not get applied to the output image. I think this is because the padding amounts are hardcoded in on lines 35-36 of padding.py. This could be fixed by setting all these padding variables equal to the input argument 'width'
Lines 7-11 of pyproject.toml do not have project specific information filled out and are causing some broken links on testpypi
The documentation on readthedocs seems to be out of date with the actual package documentation. The documentation looks good when run locally. Might just need to be synched with readthedocs?
The rotate and padding functions save the output images using matplotlib's savefig() function whereas the flipping and greyscale functions use matplotlib's imsave() function. The resulting image's look a bit different and are quite a bit different in size so it might be nice to use the same output format throughout the package.

Nice work on this project. I enjoyed reviewing/playing around with it. Let me know if you have any questions about this feedback.

Steffen

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

[x] 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:

[x] A statement of need clearly stating problems the software is designed to solve and its target audience in README
- You're 99% there! Your REAMDE is missing a crucial verb explaining what you're solving: "We're looking to simply some of the images proccessing features."
[x] Installation instructions: for the development version of package and any non-standard dependencies in README
- The instructions in your README are clear. Unfortunately, pip fails at the moment (see below).
[ ] Vignette(s) demonstrating major functionality that runs successfully locally
- I don't think there is a vignette or full example in your usage section yet.
[ ] Function Documentation: for all user-facing functions
- I see that you have docstrings in the source code, but for some reason not all of it is on read-the-docs. Only the flipping funtion documentation is there.
[ ] Examples for all user-facing functions
- It would be great to have some examples we can run locally. Maybe if you give code to download an image URL to download in a temp folder and then usage examples using that image?
[x] 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.
- In some places, not all authors are listed, or last names missing (see below). Some metadata fields are missing from TOML file.

Readme requirements The package meets the readme requirements below:

[x] Package has a README.md file in the root directory.

The README should include, from top to bottom:

[x] The package name
[x] Badges for continuous integration and test coverage, 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. Such a table should be more wide than high. (Note that the badge for pyOpenSci peer-review will be provided upon acceptance.)
- See comments below.
[x] 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.
[x] Installation instructions
[x] Any additional setup required (authentication tokens, etc)
[x] Brief demonstration usage
[x] Direction to more detailed documentation (e.g. your documentation files or website).
[x] If applicable, how the package compares to other similar packages and/or how it relates to other packages
[ ] Citation information

Usability

[x] The documentation is easy to find and understand
[x] The need for the package is clear
[ ] All functions have documentation and associated examples for use.
- It would be great to have some examples in the docstrings.

Functionality

[ ] Installation: Installation succeeds as documented.
- the pip installation from testPyPI does not work. See below.
[ ] Functionality: Any functional claims of the software been confirmed.
- I was not quite able to get padding() to work as intended. See further below. All the others do what they say on the tin !
[x] Performance: Any performance claims of the software been confirmed.
- Runs quickly!
[x] Automated tests: Tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
- Present, but I was not able to get them to work 😩 See below.
[x] Continuous Integration: Has continuous integration, such as Travis CI, AppVeyor, CircleCI, and/or others.
- Great job!
[x] Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
- Looks good!

For packages co-submitting to JOSS

[ ] The package has an obvious research application according to JOSS's definition in their submission requirements.

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

Review Comments

Dear Joshua, Mo, Rui, and Aishwarya,

Great job on getting a successful CI build and deployment! I enjoyed reviewing your work. Opening the sample image and seeing a happy lamb brought a smile to my face! 🐐 😄

Installation

I tried following package installation instructions in the README, but sadly it fails in an empty Python 3 environment:

$ conda create -n peer-review python=3 poetry -y
$ conda activate peer-review
$ pip install -i https://test.pypi.org/simple/imageprocessor
ERROR: You must give at least one requirement to install (see "pip help install")

I believe you can fix this by adding the --extra-index-url option and pointing it to the normal PyPI. Would be great to add that to your README:

pip install -i https://test.pypi.org/simple --extra-index-url https://pypi.org/simple imageprocessor

Tests:

Great job on setting up lots of tests!

Unfortunately, I was not able to run the test suite. 😢 Here is what I get:

# Confirm working directory
$ pwd
/Users/raf/Dropbox/mds/b5/524_csd/imageprocessor

# Set up an fresh environment and install
$ conda create -n peer-review python=3 poetry -y
$ conda activate peer-review
$ poetry install

# Run pytest
$ poetry run pytest

FileNotFoundError

[Errno 2] No such file or directory

at ~/opt/miniconda3/envs/peer-review/lib/python3.9/os.py:607 in _execvpe
   603│         path_list = map(fsencode, path_list)
   604│     for dir in path_list:
   605│         fullname = path.join(dir, file)
   606│         try:
→  607│             exec_func(fullname, *argrest)
   608│         except (FileNotFoundError, NotADirectoryError) as e:
   609│             last_exc = e
   610│         except OSError as e:
   611│             last_exc = e

I see a lot of tests for exception-catching, which is great!
This test that is commented out, but seems quite important, since it is meant to be an affirmative test that checks and expected output for a known input.

Usage:

flipping():
- It works on my machine! 🎉
- I was surprised that flipping() changes a colour image to greyscale. Any reason for this? I see that this is mentioned in the README, but not in the docstring.
- Great defensive block!
- I would remove comment-out code.
greyscale():
- It works on my machine! 🎉 Exactly as expected.
- Any reason that these except blocks don’t have a raise?
padding():
- It runs without error and writes to disk, but it was not the output I expected. When I run padding("sample.jpg", 10, "abc1.jpg") and padding("sample.jpg", 50, "abc2.jpg"), I get the same output. But I was expecting the second to have a lot more padding. I think these lines of code need to be changed to take width rather than 1.
- I was not expecting the resulting image to have x- and y-axis ticks and labels. Any way to remove these (I think Steffen may have suggested a solution)?
- I was expecting the output to be of the same dimensions (pixel width/height) as the image input, but that doesn’t seem to be the case. Any way to get this behaviour, or document it?
rotate():
- It works on my machine! 🎉
- Great code comments! 🙏
- It would be great to add to the rotate() docstring to explain that rotation is counter-clockwise.
- I was not expecting the resulting image to have x- and y-axis ticks and labels.

Code Style and Implementation

I see this line np.dot several times in the code, but couldn’t easily understand what it was doing. Any chance we can add a code comment here to explain?
Maybe you could DRY out your defensive checks of inputs by writing a helper function that you call in each of the four other functions?
Some of your comments are not indented (e.g. here)
Could you import only the module functions needed, instead of the entire module?

Documentation:

I only see documentation for the flipping module on Read-the-docs. However, you have other docstrings written. Something is might not be working in the Github Action...
I only see Rui’s name in the copyright info. Was that intentional? I don’t see anything about it in your license discussion but maybe this was agreed upon internally?
I suggest adding some examples to the docstrings (maybe with a URL to an image).
I suggest making your Github repo description a little more informative than “DSCI 524: Collaborative Software Development”, and to add the link to your read-the-docs page in the website field.
Since your package is all about image transformations, it would be wonderful to see some examples that embed them in your README section (before/after images).
Did you take the sample.jpg sample photo yourself? I did not see any attribution for its author and it would be great to give them credit :)
Release badge image link is broken in the README.
Codecoverage badge does not work. At the moment it points to Rui’s account, not the MDS-UBC organizational account.
It would be useful to mention the Python version to the dependencies. Perhaps also in the installation instructions.

TOML file:

I would recommend providing full names in the TOML file here.
The homepage, repo, and documentation links do not point to your repo

Version Control:

I do not see a .gitignore file. Several meta-files get committed to version control, like:
- .coverage
- coverage.xml
- The /dist folder

Great job with the 🐍 📦 . Although I put a lot of remarks above, feel free to take what's useful and leave the rest. You're not too far off from being able to tick some of the boxes. 🕺

Keep up the great work! If there's anything else I can do to help, let me know!

-Raf

Much appreciated @spentelow and @rtaph for your comprehensive and thoughtful feedback. We've learned a lot from it 👍 Hopefully, we could manage to address them all in the rest of the week. Again, thank you so much.

UBC-MDS / software-review-2021