Paul whitfield

[PaulWhitfield]

Description

Related Issue

Example

[KevinShook]

Hi Paul, I've tried the code and it works apart from 1 error that needs to be fixed:

 > base::assign(".ptime", proc.time(), pos = "CheckExEnv")
   > ### Name: ch_axis_doy
   > ### Title: Generates the x axis for day of year Produce a date axis
   > ###   starting in a specific month.
   > ### Aliases: ch_axis_doy
   > 
   > ### ** Examples
   > 
   > ch_axis_doy(wyear = 1)  # starts in January
   Error in axis(side = 1, at = wday, labels = wtxt, line = 0, tck = -0.025,  : 
     plot.new has not been called yet
   Calls: ch_axis_doy -> axis
   Execution halted

I was thinking of a way in which we could avoid needing to have a global variable, by taking advantage of R's loose typing.

We could change the function definition statement to be: ch_binned_MannWhitney <- function(DF, step, range1, range2, ptest=0.05, variable="discharge", metadata = "HYDAT_list") {

If the metadata is specified as a variable, then it will be used. If it is left as the default string then we'll just use the built-in dataframe:

  if (metadata == "HYDAT_list") {
    data("HYDAT_list")
    metadata <- HYDAT_list
  }
    sname <- ch_get_wscstation(sID, metadata = metadata)

What do you think of that? I think it solves the problem, makes the function just as easy to use and allows for the user to override, while avoiding global variables.

[KevinShook]

Actually my if statement won't work, as the type is changing. We could make the default NULL, and have that make it use the built in data, or we could say

if (typeof(metadata) == "character") 
...

[boshek]

I would suggest using the more canonical inherits(metadata, "character") to check for class.

[PaulWhitfield]

new updates

[PaulWhitfield]

putting HYDAT_list into "" worked at my end. Two failing functions corrected. One was a plot function so it wouldn't pass RCMD. The other needed the datafile.

also: == Failed tests

-- Error (test_ch_fdcurve.R:5:3): ch_fdcurve correctly returns values

Error: unused argument (title = "Station")

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 2 ] Error: Test failures Execution halted

the changes I made to the function removed the need for 'title=' so that needs to be fixed at "test_ch_fdcurve"

On Tue, 6 Apr 2021 at 14:16, KevinShook @.***> wrote:

Actually my if statement won't work, as the type is changing. We could make the default NULL, and have that make it use the built in data, or we could say

if (typeof(metadata) == "character") ...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/CSHS-CWRA/CSHShydRology/pull/23#issuecomment-814411279, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGFB73BNUFLLOMW4IRFGQWLTHNT3NANCNFSM42PLWJEA .

[KevinShook]

It doesn't work for me. When I try to run ch_binned_MannWhitney I get an error message: Error: $ operator is invalid for atomic vectors

It happens when I try to run the line sname <- ch_get_wscstation(sID, metadata = metadata)

I also get another error :

   The error most likely occurred in:

   > base::assign(".ptime", proc.time(), pos = "CheckExEnv")
   > ### Name: ch_cut_block
   > ### Title: Allows the user to extract a specific time period from a longer
   > ###   record.
   > ### Aliases: ch_cut_block
   > 
   > ### ** Examples
   > 
   > data(W05AA008)
   > subset <- ch_cut_block(W05AA005,"2000/01/01", "2010/12/31")
   Error in ch_cut_block(W05AA005, "2000/01/01", "2010/12/31") : 
     object 'W05AA005' not found

Not sure why

[PaulWhitfield]

Kevin:

I did think yesterday that changing metadata = "HYDAT_list" was not a sensible solution as we would never use quotes in that way. While it fixes the Note regarding global variable when the code executes, it treats "HYDAT_list" as a character string and results in the error $ operator is invalid for atomic vectors

The second error was a simple fix. The data is W05AA008 not W05AA005 This has been corrected.

New versions have been pushed but the build generates NOTE ch_binned_MannWhitney: no visible binding for global variable 'HYDAT_list' ch_fdcurve: no visible binding for global variable 'HYDAT_list' ch_flow_raster: no visible binding for global variable 'HYDAT_list' ch_flow_raster_qa: no visible binding for global variable 'HYDAT_list' ch_flow_raster_trend: no visible binding for global variable 'HYDAT_list' ch_get_wscstation: no visible binding for global variable 'HYDAT_list'

I found this suggestion:

"For our package, as there are literally thousands of variables to list in this file, it makes it very difficult to maintain this list and makes the file very long. If, however, the variables belong to data which are stored within your package then this can be greatly simplified to globalVariables(names(my_data)) "

So we could simply add

globalVariables(names(HYDAT_list) to the appropriate file.

That looks to be the only outstanding issue at my end. P

On Tue, 6 Apr 2021 at 16:21, KevinShook @.***> wrote:

It doesn't work for me. When I try to run ch_binned_MannWhitney I get an error message: Error: $ operator is invalid for atomic vectors

It happens when I try to run the line sname <- ch_get_wscstation(sID, metadata = metadata)

I also get another error :

The error most likely occurred in:

base::assign(".ptime", proc.time(), pos = "CheckExEnv")

Name: ch_cut_block

Title: Allows the user to extract a specific time period from a longer

record.

Aliases: ch_cut_block

** Examples

data(W05AA008)

subset <- ch_cut_block(W05AA005,"2000/01/01", "2010/12/31")

Error in ch_cut_block(W05AA005, "2000/01/01", "2010/12/31") :

 object 'W05AA005' not found

Not sure why

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/CSHS-CWRA/CSHShydRology/pull/23#issuecomment-814473664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGFB73GPH3TLKS6LLBKRWLDTHOCPDANCNFSM42PLWJEA .