Input limits
Closed this issue · 20 comments
Our rapport docs page it says that "number" etc. input fields's limit deals with the number of passed elements. This is strange, and I have must forgot something really obvious, please recall why we needed this kind of limit.
But even if this is OK, the result seems to be strange:
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[1] | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=3)
+ + + + + + > Error in stopf("input \"%s\" has length of %d, and should be %s", name, :
input "x" has length of 3, and should be 1
So it seems that we rather check the passed one number if it's inside the limited interval, which makes more sense anyway. So e.g. I could create a template which would take one number as parameter between e.g. 1 and 10, which is working now (although the docs should be updated):
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[1,10] | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=3)
+ + + + + + > _3_
But the current implementation does not allow users to specify the limits below 1. E.g. I would want to ask users to pass a number between 0 and 100:
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[0,100] | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=3)
+ + + + + + > Error in check.limit(gsub(re5, "\\3", x), "number") :
only positive integers should be provided as a limit
But I might even want to specify the limits as negative numbers.
Anyway, a quick fix would be really welcomed. @aL3xa: pls think about the limit specification and pls also fix the "number" input type to let users pass any numbers there (e.g. 0.231234), which latter is a really high priority for rapporter.net.
I made the change in input specification (without updating the documentation, bravo me!) in this one. OK, note taken, checking it out...
Just to add one more thingy. This commit fixed that nasty options default value bug. Prior to this commit, rapport:::check.type("fee, fi, foo, fam") yeilded:
$type
[1] "option"
$limit
$limit$min
[1] 1
$limit$max
[1] 1
$default
[1] "fee" "fi" "foo" "fam"
$mandatory
[1] FALSE
and after the fix it returns just what it's supposed to:
$type
[1] "option"
$limit
$limit$min
[1] 1
$limit$max
[1] 1
$default
[1] "fee"
$mandatory
[1] FALSE
Yay!
Thanks @aL3xa.
I have just checked my above example, please verify:
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[1,10] | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=3)
> + + + + + + > Error in if (is.na(default)) default <- NULL : argument is of length zero
The offending line is somewhere [L667](aa6acaa#L0R667%5D (maybe that should be an ifelse), but I just do not want to mess your code.
Minimal reproducible example:
> rapport:::check.type("number[-100,100]")
Error in if (is.na(default)) default <- NULL : argument is of length zero
So if you do not pass a default value, it fails. Is it normal?
The bugger was that I didn't check for NULL. =/
Awesome, thanks. Wanna check out this relevant problem? :)
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[-100,10]=3 | x | x
y | number[-100,10] | y | y
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=-10.534)
+ + + + + + + > Error in stopf("\"%s\" is not of \"%s\" type", val, input.type) :
In short: if a user does not pass a parameter which is not required and has no default value, then it fails.
OK, my fixes should be classified as EPIC FAIL
Come on! We are pretty close :)
I know you'd kill me, but I still get:
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[-100,10]=3 | x | x
y | number[-100,10] | y | y
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=-10.534)
+ + + + + + + > Error in stopf("\"%s\" is not of \"%s\" type", val, input.type) :
Like I wrote above: if a user does not pass a parameter which is not required and has no default value, then it fails.
Do I miss something obvious?
Ah, and thanks for the tests! Works like a charm. Although we have some problems with other tests:
> test_package('rapport')
rapport : ............1......2
Limit specifications : .................
Skewness tests : .
Kurtosis tests : .
As you are already tweaking the fn, could you pls also check this out:
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[1,10] | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=10.534)
+ + + + + + > Error in sprintf(s, ...) :
invalid format '%d'; use format %f, %e, %g or %a for numeric objects
The error is triggered as I passed a non-integer number, which is higher then max. limit.
If the number is a whole number and higher then limit, the error msg is prettier:
> rapport(tpl,x=11)
Error in stopf("input \"%s\" has length of %d, and should be %s", name, :
input "x" has length of 11, and should be between 1 and 10
Hm, and did you run test_package("rapport")? :)
> rapport("example", ius2008, v = "age", s = "FOO BAR")
Error in stopf("input \"%s\" has length of %d, and should be %s", name, :
input "s" has length of 7, and should be 1
Okay, so we opened up a pandora's box with a bugfix.
Yay!
IMO, this is a completely different bug, but let's continue discussion here.
One more reflection on input specification crappiness: if you have number[3]=4, what then? =)
Thanks @aL3xa.
My last comment here:
> test_package('rapport')
rapport : .....................
Input limit specifications : .................
Input type specifications : .........1.
Skewness tests : .
Kurtosis tests : .
1. Failure: should provide correct input definition ----------------------------
rapport:::check.type("string") not equal to list(type = "string", limit = list(min = 1, max = 1), default = NULL, mandatory = FALSE)
Component 2: Component 2: Mean relative difference: 255
Error: Test failures
I lied. This is my last comment here :)
> tpl <- strsplit('<!--head
Title: test
Description: test
Author: test
x | number[-100,10]=3 | x | x
head-->
<%=x%>', '\\n')[[1]]
rapport(tpl,x=-10.534)
+ + + + + + > Error in FUN(X[[1L]], ...) :
number input "x" (value: -10.534) should fall in interval [-100, 10]It's not just about negative numbers, but:
> rapport(tpl,x=1.1)
Error in FUN(X[[1L]], ...) :
number input "x" (value: 1.1) should fall in interval [-100, 10]
whattaf... O_o
OK, I'll check that when I get back home. =/
That's what I call a professional reply from the dev team :)
All right, thanks Alex.
Maybe I miss something with option type, please verify:
> tpl <- strsplit('<!--head
Title: A minimal number test
Description: foobar
Author: Gergely Daróczi (@DG)
o | foo,bar,box | o | foobar
head-->
<%=o%>', '\\n')[[1]]
rapport(tpl, o="bar")
+ + + + + + > Error in match.arg(input.value, input.default) :
'arg' should be one of “foo”
No, thanks God (and you of course).
But pls do not forget to update the GH page.