search

dahtah / imager

R package for image processing
GNU Lesser General Public License v3.0
187 stars 43 forks source link

fixed_NOTE_rename #104

Closed ShotaOchi closed 4 years ago

ShotaOchi commented 5 years ago

I fixed the NOTE "checking R code for possible problems".

index.coord: no visible global function definition for ‘rename’ Undefined global functions or variables: rename

I added "plyr" to the imports field of the DESCRIPTIOn file, and added a new line "#' @importFrom plyr rename' to the cimg_class.R file.

Ax3man commented 4 years ago

It's probably better to rewrite that line to avoid the plyr dependency altogether. See e.g. here.

ShotaOchi commented 4 years ago

Do you mean it's better to rewrite the code of index.coord function shown below instead of adding plyr package dependency? https://github.com/dahtah/imager/blob/39b10fe2dea36b12d2cf12100f65babd157ffb4a/R/coordinates.R#L121

Ax3man commented 4 years ago

Yes, you could e.g. replace it with

names(coords)[names(coords) == "c"] <- "cc"

One advantage of using plyr::rename in this situation though, is that it will throw a warning if there already is a column named "cc" in coords. So if that is a legitimate concern, you may want to add a check for that.

Ax3man commented 4 years ago

I see that in other locations similar code was replaced with:

names(coords) <- stringr::str_replace_all(names(coords),c("cc"="c"))

This one was probably just overlooked.

ShotaOchi commented 4 years ago

Which means we'd better rewrite the code as shown below for consistency with the similar code in other locations?

names(coords) <- stringr::str_replace_all(names(coords),c("c"="cc"))
ShotaOchi commented 4 years ago

I recreated a pull request #111 . That's why I close this pull request.