search

UBC-MDS / software-review-2022

0 stars 0 forks source link

Submission Group 09: ImgHelpR #15

Open jasmineortega opened 2 years ago

jasmineortega commented 2 years ago

Submitting Author Name: Sufang Tan, Jasmine Ortega, Ho Kwan Lio, Maeve Shi. Submitting Author Github Handle: Sufang Tan @Kendy-Tan, Jasmine Ortega @JasmineOrtega, Ho Kwan Lio @stevenlio88, Maeve Shi @MaeveShi

Repository: https://github.com/UBC-MDS/ImgHelpR Version submitted: Submission type: Standard Editor: Reviewers: Paniz Fazlali @paradise1260, Melisa Maidana @mmaidana24318), Lianna Hovhannisyan @liannah

Package: ImgHelpR
Title: Tools to Help Process Image in R
Version: 0.1.0
Authors@R: 
    c(person(given = "Sufang",
             family = "Tan",
             role = "aut"),
      person(given = "Jasmine",
             family = "Ortega",
             role = "aut"),
      person(given = "Ho Kwan",
             family = "Lio",
             role = "aut"),
      person(given = "Maeve",
             family = "Shi",
             role = c("aut", "cre"),
             email = "first.last@example.com",
             comment = c(ORCID = "YOUR-ORCID-ID")))
Description: ImgHelp is a simple R library 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.
License: MIT + file LICENSE
URL: https://github.com/UBC-MDS/ImgHelpR
BugReports: https://github.com/UBC-MDS/ImgHelpR/issues
Depends:
    R (>= 3.0.2)
Imports:
    jpeg (>= 0.1.9),
    R.utils,
    testthat
Encoding: UTF-8
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.1

Scope

ImgHelpR is a simple image manipulation tool. It allows users to read in JPEG images into R and rotate, crop, color convert, or compress the image.

The target audience are R users who want a simple introduction to image processing tools in R. Scientific applications could include manipulating images for model training, for research papers, etc.

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

MEE Options - [ ] The package is novel and will be of interest to the broad readership of the journal. - [ ] The manuscript describing the package is no longer than 3000 words. - [ ] You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see [MEE's Policy on Publishing Code](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/journal-resources/policy-on-publishing-code.html)) - (*Scope: Do consider MEE's [Aims and Scope](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/aims-and-scope/read-full-aims-and-scope.html) for your manuscript. We make no guarantee that your manuscript will be within MEE scope.*) - (*Although not required, we strongly recommend having a full manuscript prepared when you submit here.*) - (*Please do not submit your package separately to Methods in Ecology and Evolution*)

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:

Functionality

Estimated hours spent reviewing: 1.5h

Review Comments

Nice work team! The package is really interesting. I have a few comments:

  1. I tested the examples in vignette. ImgRotate(ubc, 180) did not output a 180-rotated image. You are probably in the middle of updating the repo, so you can take a look at that.
  2. Also, ImgCompress(ubc, method = "resize", level = 1) returned an error saying 'subscript out of bounds'. I am not sure what caused that error but you might want to address that.
  3. In addition, Crop(ubc, 'hello', 'world') returns the developer-defined error 'ValueError: Desired height cannot exceeds original height'. The error message can be changed to something different like 'width and height have to be integers'.
  4. Having brief examples in the README.md file can be helpful for people who want to quickly get an idea of what package does through simple examples.
  5. The name of the functions are very informative.

Keep up the good work!

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:

Functionality

Estimated hours spent reviewing: 1 hour

Review Comments

Amazing job. The package is very well written and the installation was smooth, and everything ran.

  1. Readme file is very well written, however, the Usage section lacks any examples of how the functions can be used. It will be nice to add this example usage, as it will help users to get acquainted with the package from the first glance.
  2. The documentation is very well written, however, The roxygen param definition format is different for Corp.R file. It uses @param --- definition, while others use simply@param definition` format. It is a minor thing, but I think it will be better to have consistency in all roxygen comments.
  3. In Crop function, the text says Value Error: Height and width for the desired image must be integer, however, you are checking modulo 1, thus if I pass 1.0 (double), I should expect an error, however, no error will rise: 1.0 %% 1 == 0 is True . Instead, you can check that the type of the passed argument is an integer, or if that is not very important, you can modify the error message.
  4. Error messages are very informative, however, the error messages are not consistent through all files. The Crop function uses colon format, while all others use comma format after the type of error. I think it will look better for the user to have the same format for all the functions and error messages.
  5. In documentation file, one email is present with email = "first.last@example.com" value. I think it will be better to either remove it or include the working email, so users will be able to contact if need be.

I enjoyed reviewing the package, and everything worked as expected. Great idea, and great implementation. Congratulations on creating a wonderful package and keep up the good work.

mmaidana24318 commented 2 years ago

Hi team. Please find my comments below.

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:

Functionality

Estimated hours spent reviewing: 2 hrs

Review Comments

  1. Vignettes - 2 functions explain the parameter img as "=input image from package jpeg" and the other 2 as "input image"
  2. All the examples documented use a random matrix and there are no instructions on how to use the package with a real image. It took me sometime to set up the jpeg package and learn how to plot the image with it. It would be nice if you could add a few lines to show a useful end to end example specially because this is designed for beginners.
  3. I think something is not working properly with ColoConv and the parameter "gray". I was able to run it and plot the modified image for all the other colors, but the gray option throws an invalid 'xlim' value when trying to plot it.
  4. ImgRotate works only with square images. The image gets completely distorted if rectangular.
  5. You may want to consider adding some tests for rectangular images.

Very interesting package! I really enjoyed testing the different functions.