Gh 2758 fix tidyverse

[harshagr70]

Fixes a part of issue #2758 ,
##Updated the .R files to fix no visible binding errors for variables .
##Updated reference.log file .
##Added import for .data
Remaining - variable "year" still accounts under the no visible binding call , the only part where i found it to be referenced is inside the for loop , @infotroph need your suggestion on fixing the issue for this variable .

[harshagr70]

also thoughts on how the mutate is being used twice here , which may be overwriting the Date !!

[infotroph]

You're right that it's overwriting, but that's OK. The first call sets the date with no hour set (I think technically this gets treated as exactly midnight, but datetimes are complicated and I could be wrong), then the second call converts the result of the first call so that it includes the hour (and minute, but that's set to zero).

I don't know exactly what the previous author did it in two steps and bet it'd be possible to find a cleaner way to write it, but given this is a maintenance fix I think you're right to just add the .data and leave the rest of the line as it is.

[infotroph]

Nice change! I think this makes the inner paste0 redundant too:

Suggested change

	paste0(formatted_start,
	"-",
	formatted_start, "-", formatted_stop, ".clim")

[harshagr70]

correct !! , i have already removed it to check the redundancy thing ! i will push the changes

[infotroph]

Suggested change

formatted_stop), ".clim")

(Only if accepting the suggestion above, of course)

[infotroph]

You're right that it's overwriting, but that's OK. The first call sets the date with no hour set (I think technically this gets treated as exactly midnight, but datetimes are complicated and I could be wrong), then the second call converts the result of the first call so that it includes the hour (and minute, but that's set to zero).

I don't know exactly what the previous author did it in two steps and bet it'd be possible to find a cleaner way to write it, but given this is a maintenance fix I think you're right to just add the .data and leave the rest of the line as it is.

[infotroph]

variable "year" still accounts under the no visible binding call , the only part where i found it to be referenced is inside the for loop

I think everything in that loop should be OK -- looks like the one it's complaining about is the subset on line 143 of model2netcdf. Maybe this would work:

--- a/models/sipnet/R/model2netcdf.SIPNET.R
+++ b/models/sipnet/R/model2netcdf.SIPNET.R
@@ -140,7 +140,7 @@ model2netcdf.SIPNET <- function(outdir, sitelat, sitelon, start_date, end_date,
     print(paste("---- Processing year: ", y))  # turn on for debugging
 
     ## Subset data for processing
-    sub.sipnet.output <- subset(sipnet_output, year == y)
+    sub.sipnet.output <- sipnet_output[sipnet_output$year == y,]
     sub.sipnet.output.dims <- dim(sub.sipnet.output)
     dayfrac <- 1 / out_day
     step <- utils::head(seq(0, 1, by = dayfrac), -1)   ## probably dont want to use

[harshagr70]

Thankyou !! @infotroph for suggestions and review , on it and i will push the changes incorporating the suggestions !!

[harshagr70]

@infotroph I have made changes as per the suggestions and also applied the following to ensure smooth operation:

*Edited DESCRIPTION to add rlang to the Imports section
*Rebuilt the Roxygen docs using devtools::document() to update NAMESPACE
*Ran ./scripts/generate_dependencies.R to update docker/depends/pecan_package_dependencies.csv

While doing this, I noticed that devtools::document() created some duplicate files (with 2 appended to their names). I believe these are safe to remove, but I’d like your thoughts on this before proceeding. What are your views?

[infotroph]

I believe these are safe to remove

Yep, I agree. Those look to me like they came from some kind of local hiccup, and either way we don't want them in this PR. Do they come back if you git rm them and run Roxygen again?

[harshagr70]

@infotroph i have checked , it is not creating redundant files on running again as i checked , i have removed those copied files .

[infotroph]

@harshagr70 Thanks, this now looks good. Do you want to tackle the year in model2netcdf.SIPNET as part of this PR or should I merge this now and we come back to that later?

[harshagr70]

@infotroph Its already being tackled in this PR , i forgot to update the reference.log , now its well updated i have fixed that part as well !!