Fix is.vector()

[lgatto]

This line uses is.vector() to check if a variable is an atomic vector, not only a vector (including a list):

> is.vector(1:5)
[1] TRUE
> is.vector(list())
[1] TRUE
> is.vector(1:5, "numeric")
[1] TRUE
> is.vector(list(), "list")
[1] TRUE

The solution here is the also specify a mode:

> is.vector(1:5, "numeric")
[1] TRUE
> is.vector(list(), "list")
[1] TRUE
> is.vector(1:5, "list")
[1] FALSE

I would suggest to use

• !is.vector(list(), "list")
• !is.list()

in such cases.

For that specific code chunk, there's no final else. We could probably test for

1. is.list(.)
2. inherits(., "List")
3. assume the rest are atomic vectors.

@jorainer @sgibb - what do you think?

[jorainer]

Is there a problem with the original code or why do you suggest to change this?

[lgatto]

I have not observed an issue with the original code.

I just wanted to highlight that is.vector() doesn't test if we have an atomic vector. It doesn't matter here, given that we check if it's a list first, so it implicitly test for an atomic vector.

And while looking at the code, I thought that it could be re-written with a final else. But it doesn't have to be changed; I don't think there's a but - at least in the anticipated use-cases :-)

I thought that additional pairs of (critical and expert) eyes should look at it.

[jorainer]

I'm OK with the original code - it looks clean/clear and easy to understand. But I'm also OK with changing it is you think it is necessary? Generally I would prefer is.list and similar instead of their negation !is.list - but that's maybe just my poor brain that has sometimes problems thinking around negations, double negations etc.