UBC-MDS / software-review

MDS Software Peer Review of MDS-created packages
1 stars 0 forks source link

Submission:pyimager #46

Open dataubc opened 4 years ago

dataubc commented 4 years ago

Submitting Author: Keanna Knebel (@Keanna-K ), Mohammed Salama (@dataubc),Zhengyang (@zoepan00 ) Pan, Haoyu (@clsu22) Package Name: pyimager One-Line Description of Package: The pyimager package contains functions that aid in image manipulation and processing. Repository Link: https://github.com/UBC-MDS/pyimager Version submitted: 1.1.0 Editor: Varada kolhatkar @kvarada) Reviewer 1: Sakariya Aynashe(@eyrakas) Reviewer 2: Holly Williams(@hwilliams10) Archive: TBD
Version accepted: TBD


Description

Scope

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

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](https://joss.readthedocs.io/en/latest/submitting.html#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](https://joss.readthedocs.io/en/latest/submitting.html#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](https://joss.readthedocs.io/en/latest/submitting.html#what-should-my-paper-contain) 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.

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

eyrakas commented 4 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:

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


Review Comments

General Comments

Thank you for giving me the opportunity to provide some review on your amazing project. The README of your project greatly outlines all the installation requirements, dependencies and steps to be taken to successfully run your package. That was great and I was able to follow the guidelines without any issue. Personally, I liked all the functions which happen to be doing diverse functionality that fall under the same domain of image handling. Your function like circropper was elegantly designed and produces the desired circular image as the output. I have also tested the other functions such as redusize, reducolor and imgfilter and they all generate the desired output. As per the expectations outlined in your project repo, the test cases including some essential edges pertaining to function inputs have all been well taken care of. That is something I want to personally compliment to the team. Glad to see that you have also chosen a publicly available image for your testing to avoid any legal implications of having to go through tedious permissions for personal or licensed images. There are very minor aspects you may consider as an improvement if time permits, which is indicated in the specific comments below.

Specific Comments

hwilliams10 commented 4 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:

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 hrs


Review Comments

Overall

Great package idea! I can see how this would be useful, and it is pretty sleek that all your functions only require one line.

I was able to install and run your package without any issues following your instructions in the README. Each function also performed as expected, and I even tried them out on my own image and everything still worked great!

Your test coverage is good and I did not get any warnings or errors.

I also liked that you included dependency versions in your README. It made it easy for me to quickly check and update required packages before installing your package.

Suggestions

This section contains some suggestions that you can choose to implement or ignore, depending on your group's preferences and available time.

General:

Function-specific:

Last (very minor) suggestion:

Closing

Overall, great job. Thanks for letting me review your package. I'll probably use it in the future!

zoepan00 commented 4 years ago

Hi, thank you for your feedback Sakariya(@eyrakas) and Holly(@hwilliams10). We created issues regarding your guys reviews. Please refer to Sakariya feedback and Holly feedback for details tracking how we addressed the suggestions. Please feel free to leave you comments if anything comes up.