Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

future:::tweakFutureAssignmentCall(): Warning message: In if (n != 3) return(expr) : the condition has length > 1 and only the first element will be used #395

Closed
HenrikBengtsson opened this issue Jul 28, 2020 · 2 comments
Milestone

Comments

@HenrikBengtsson
Copy link
Owner

From DavisVaughan/furrr#115 (comment):

@HenrikBengtsson, if you want to handle this specific case in future, it comes from future:::tweakFutureAssignmentCall() making the perfectly reasonable assumption that length() returns something of length 1. I imagine that you will have to handle the possibility that length() could return something that isn't size one in more than just this one specific function for it to be a bulletproof fix, which might not be worth it.

fi <- Formula::Formula(Sepal.Length~Sepal.Width)

# Comes from here...
gp <- future::getGlobalsAndPackages(fi)
#> Warning in if (n != 3) return(expr): the condition has length > 1 and only the
#> first element will be used

# Which calls this:
globals:::findGlobals(fi, tweak = future:::tweakExpression)
#> Warning in if (n != 3) return(expr): the condition has length > 1 and only the
#> first element will be used
#> [1] "~"            "Sepal.Length" "Sepal.Width"

# tweakExpression() uses tweakFutureAssignmentCall() which makes the
# perfectly reasonable assumption that `length(expr)` returns
# something of length 1
future:::tweakFutureAssignmentCall(fi)
#> Warning in if (n != 3) return(expr): the condition has length > 1 and only the
#> first element will be used
#> Sepal.Length ~ Sepal.Width

This is because:

> Formula:::length.Formula
function (x) 
{
    c(length(attr(x, "lhs")), length(attr(x, "rhs")))
}
<bytecode: 0x55f9d6ee5728>
<environment: namespace:Formula>

Hmm... yeah, that's not what you'd expect from length() but maybe there is a way to avoid the assumption that length() returns something else than a numeric/integer scalar.

@DavisVaughan
Copy link

There is even a warning about this in ?length

Package authors have written methods that return a result of length other than one (Formula) and that return a vector of type double (Matrix), even with non-integer values (earlier versions of sets). Where a single double value is returned that can be represented as an integer it is returned as a length-one integer vector.

HenrikBengtsson added a commit that referenced this issue Jul 31, 2020
@HenrikBengtsson
Copy link
Owner Author

Fixed in the develop branch. The other "tweak" functions had similar issues - I think the only safe, generic solution is to look at length(unclass(expr)), so I went with that. Thxs again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants