ices-eg / WK_RDBES

3 stars 6 forks source link

Draft function - doDBEestimantionObjUpp #55

Open davidcurrie2001 opened 3 years ago

davidcurrie2001 commented 3 years ago

This issue will describe any comments and discussion relating to Kirsten's draft function "doDBEestimantionObjUpp"

davidcurrie2001 commented 3 years ago

Issues identified by running devtools::check()

Style issues identified by running _lintr::lint_package(linters=lintr::with_defaults(object_name_linter=lintr::object_namelinter(styles = "camelCase")))

davidcurrie2001 commented 3 years ago

For interest I've put a version of the function that I manually cleaned at https://github.com/ices-eg/WK_RDBES/blob/SG6/WKRDB-EST2/subGroup6/icesRDBES/R/doDBEestimantionObjUpp.R

KirstenBirchHaakansson commented 3 years ago

So you are solving all the problems we have created by not following the style?

From: David Currie notifications@github.com Sent: 17 September 2020 13:54 To: ices-eg/WK_RDBES WK_RDBES@noreply.github.com Cc: Kirsten Birch Håkansson kih@aqua.dtu.dk; Assign assign@noreply.github.com Subject: Re: [ices-eg/WK_RDBES] Draft function - doDBEestimantionObjUpp (#55)

For interest I've put a version of the function that I manually cleaned at https://github.com/ices-eg/WK_RDBES/blob/SG6/WKRDB-EST2/subGroup6/icesRDBES/R/doDBEestimantionObjUpp.R

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/ices-eg/WK_RDBES/issues/55#issuecomment-694181043, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGESPOTIDEGLFOX34LMFVK3SGH2GDANCNFSM4RQHLP6A.

davidcurrie2001 commented 3 years ago

@KirstenBirchHaakansson don't get used to it :-)

davidcurrie2001 commented 3 years ago

Another issue I noticed was that if you write R code as text strings and then call eval to run it I don't think that devtools recognises any package dependencies e.g. your select in the function doesn't trigger devtools to tell me to add dplyr to the imports list - this could cause problems when people try and install our package but don't have dplyr installed.

KirstenBirchHaakansson commented 3 years ago

Don’t know how to change the eval parts, but the script only use dplyr ones (I think) and I have added dplyr:: to that part when migrating the script to SG7 WK_RDBES\WKRDB-EST2\subGroup7\funs

I will replace the use of dplyr in this script

From: David Currie notifications@github.com Sent: 17 September 2020 15:24 To: ices-eg/WK_RDBES WK_RDBES@noreply.github.com Cc: Kirsten Birch Håkansson kih@aqua.dtu.dk; Mention mention@noreply.github.com Subject: Re: [ices-eg/WK_RDBES] Draft function - doDBEestimantionObjUpp (#55)

Another issue I noticed was that if you write R code as text strings and then call eval to run it I don't think that devtools recognises any package dependencies e.g. your select in the function doesn't trigger devtools to tell me to add dplyr to the imports list - this could cause problems when people try and install our package but don't have dplyr installed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ices-eg/WK_RDBES/issues/55#issuecomment-694230308, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AGESPOVVYMNC7M6IXBUE3G3SGIEYXANCNFSM4RQHLP6A.

davidcurrie2001 commented 3 years ago

I have added some test for your function - see https://github.com/ices-eg/WK_RDBES/blob/SG6/WKRDB-EST2/subGroup6/icesRDBES/tests/testthat/test-doDBEestimantionObjUpp.R