anhoej / qicharts2

R package: Quality improvement charts
38 stars 12 forks source link

"Error in if (getOption("qic.clshade")) { : argument is of length zero" #21

Closed md0u80c9 closed 5 years ago

md0u80c9 commented 5 years ago

Hi,

I've just upgraded to qicharts2 0.60 from CRAN (from whatever the previous CRAN release was). I had a fairly straightforward chart plotted with the following code which was working yesterday:

qicharts2::qic(ReportPeriod, TCNoBrainImagingWithin1hrD, TCn72hrs,
  data     = ssnapScores, target = 0.5,
  chart    = 'p',
  title     = 'Proportion not receiving brain imaging within 1hr',
  xlab     = 'Month')

I've tried it today and I get the error message in the title. I'm assuming that the error is suggesting that qic.clshade is NULL, which is clearly an internal function. I can't see any changes to my code - so I don't think I've broken it myself.

Any thoughts what may be going on?

md0u80c9 commented 5 years ago

Aha - I may have cracked the 'why it happens'. I'm writing code within a package (helper function for a shiny app) without using library(qicharts2). Although my code itself is fully referenced, something within the package needs library(qicharts2) to be called in order to function. If I do it from the command line and call library first - the code works fine. If I restart R and do it without calling library, it will fail.

pwildenhain commented 5 years ago

In the helper.functions.R file, there is a method defined called .onAttach. This is a special method that will execute some code when library(qicharts2) is run.

In this case it's defining some variables, which get used later on in qic() (one of them being qic.clshade). This means that if you try to run qicharts::qic(...), without loading qicharts2, then the function will throw an error, since it can't find those variable that it expects to be loaded. If you're writing code for an analysis, library(qicharts2) will solve your issue, as you've discovered.

If you're writing code for an R package however, I would caution against having library(qicharts2) in your code. You can always define those variables though, by setting them in options(qic.clshade = TRUE, etc...) before running qic() in your code

anhoej commented 5 years ago

You're absolutely right. I have stumbled on this issue myself. You need to load qicharts2 in advance. using qicharts2::qic() won't work. It's on my issues list.

md0u80c9 commented 5 years ago

You could rewrite the if statements to use the default setting if the value is NULL.

anhoej commented 5 years ago

Fixed on GitHub (using the default argument in getOption()). Please test and report.

md0u80c9 commented 5 years ago

Will do.

You could potentially add a unit test for this by duplicating the existing test file and just omitting the library() command :-).

Drew

On 22 Mar 2019, at 13:55, Jacob Anhøj notifications@github.com wrote:

Fixed on GitHub (using the default argument in getOption()). Please test and report.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/anhoej/qicharts2/issues/21#issuecomment-475629842, or mute the thread https://github.com/notifications/unsubscribe-auth/ADvnSgPNvhc0jW_gjCykL0NpISURY4lUks5vZODXgaJpZM4cDhRv.