Skip to content
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

Submission: pymagine (Python) #15

Open
11 of 22 tasks
brendoncampbell opened this issue Mar 16, 2020 · 3 comments
Open
11 of 22 tasks

Submission: pymagine (Python) #15

brendoncampbell opened this issue Mar 16, 2020 · 3 comments
Assignees

Comments

@brendoncampbell
Copy link

brendoncampbell commented Mar 16, 2020

Submitting Author: Team John McCarthy (Katie Birchard (@katieb1), Brendon Campbell (@brendoncampbell), Sukriti Trehan (@Sukriti1312), Trevor Kwan (@trevorkwan))
Package Name: pymagine
One-Line Description of Package: This package includes functions that provide several different types of visual manipulation filters to an input image.
Repository Link: https://github.com/UBC-MDS/pymagine/tree/v1.1
Version submitted: v1.1
Editor: Varada Kolhatkar (@kvarada)
Reviewer 1: Shiying Wang (@fsywang)
Reviewer 2: Saurav Chowdhury (@saurav193)
Archive: TBD
Version accepted: TBD


Description

This package includes four main functions that apply a filter to an input image as summarized below:

  • Tunnel distortion: produces an image with strong visual distortion intended to create a tunnel or pincushion effect
  • Colour filters: produces an image with different user-specified colour distortions (ex: blue tone)
  • Edge detection: identifies edges by looking at where the image brightness changes sharply, and produces a black and white image highlighting the locations of these edges
  • Vignetting: produces an image with reduced brightness around the periphery compared to the image center

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

Our image manipulation package does not clearly fall into any of the specified categories, but is closest to the data extraction and visualization groups

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

pymagine is for anyone looking to apply artistic image filter effects using a python script. It does not have a direct scientific application.

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

A variety of image processing packages providing some similar functionality already exist within the Python ecosystem, including Pillow, scikit-image, and the more advanced computer vision oriented OpenCV. The purpose of our package is to provide functions that apply some common artistic filter transformations to a given input image.

  • 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

@fsywang
Copy link

fsywang commented Mar 20, 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.

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: 2 hours


Review Comments

The README is very clear and well structure. The documentary and codes are very clear and the tests run perfectly on my local machine.

Here are some feedback and issues I found:

Documentation

  • In README, the usage of tunnel_filter should be updated to tunnel_filter("~\image.jpg", rot=-0.2) (remove the k argument).

  • In README, usage of the importing functions should agree with filter functions. For example, in order to let vignette_filter("~\image.jpg", strength=1.0, x=0.5, y=0.5) work, users should run from pymagine.vignette_filter import vignette_filter to import the function. Alternatively, change the usage of filter function to vignette_filter.vignette_filter("~\image.jpg", strength=1.0, x=0.5, y=0.5).

  • There are typos on the example part in vignette_filter docstring (vignette_filter("img/picture.jpeg", strength=2.5, a=0.25, b=0.75)). Here, a and b should be x and y.

Other minor issues:

  • There is no example in colour_filters docstring.

  • The codes are very clear, but it will be good to also include some inline comments.

  • There are two image files (colour_filter.jpeg and edge_detection.jpg) in the root of the repo. It might be better to put them in an image folder.

Functionality

  • The code from pymagine import pymagine does not work after I successfully install the function. In order to let it work, there should be a file named pymagine.py in pymagine folder.

  • Alternatively, here are some other ways to import the function:

img1

@saurav193
Copy link

saurav193 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: 1hr


Review Comments

The README is well written. The package you have built is simple and useful for altering an image. The functions and tests run fine. Good job on the coverage and the documentation of the README and the fucntions. Although There are some issues I noticed during the function run. Here is my feedback :

Major issues

  1. The functions are not imported using import pymagine as pymagine . Rather this way seems to work :
  • import pymagine.vignette_filter as vf
  • import pymagine.colour_filters as cf
  • import pymagine.tunnel_filter as tf
  • import pymagine.edge_detection as ed
    So the README or the folder structure should be updated accordingly.
  1. The tunnel_filter() does NOT have a k argument as mentioned in the README.

  2. The edge_detection() gave a completely black image when using a face image. Please check with a facial image.
    e.g.
    Virat_Kohli_portrait

edge_detection

Minor issues

  1. function names should have consistency - like "filters" in colour_filters().
  2. Doctring in vignette_filter() has argument name "sigma" inplace of the actual argument name "strength"
  3. tunnel_filter() prints "some shape" values used for debugging. which should have been removed.

Suggestions

  1. The file name of saved image can be indexed(1,2,...) with a number so it's not overwritten every time after function run.
  2. Printing the image directly using imshow() or other functions would have made more sense to me since I would be able to see the updated image directly in jupyter notebook/lab/other ide.
  3. A separate function argument for saving the output to a folder would be better.

@katieb1
Copy link

katieb1 commented Mar 26, 2020

Thank you Shiying and Saurav for you thoughtful feedback! We managed to address most of the feedback including:

  • fixing our documentation,
  • removing the random images from our root directory,
  • fixing the import code,
  • fixing the bug in the edge_detection filter, and
  • adding a separate function argument for saving files to a user-specified output folder (we also thought this would take care of the indexing suggestion, since a user could input an array of images and specify their own index in a loop).

For more a more thorough look at how we addressed the feedback, you can take a look at our GitHub issue on pymagine: UBC-MDS/pymagine#62

Also, here is the link to the final version of pymagine: https://github.com/UBC-MDS/pymagine/tree/v2.0.0

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

4 participants