-
Notifications
You must be signed in to change notification settings - Fork 360
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
Fix tests on Julia 0.7 #1405
Fix tests on Julia 0.7 #1405
Conversation
@@ -606,7 +606,7 @@ julia> df | |||
``` | |||
""" | |||
Base.filter!(f, df::AbstractDataFrame) = | |||
deleterows!(df, findall(!f, eachrow(df))) | |||
deleterows!(df, findall(collect(!f(r)::Bool for r in eachrow(df)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change necessary? Because eachrow
isn't indexable and findall
requires an indexable collection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That's kind of annoying, IIRC in the PR it was mentioned that a wrapper could be introduced in Base to use linear indices in such situations. But here it's probably faster to allocate a vector anyway (JuliaLang/julia#25879), and we do that in deleterows
.
"immutable", "do", "module", "baremodule", "using", "import", "struct", | ||
"export", "importall", "end", "else", "elseif", "catch", "finally"]) | ||
|
||
if VERSION < v"0.7.0-DEV.3220" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't this happen in 0.6-dev?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, or the tests would fail on 0.6. This commit appears to be the one which removed these deprecated identifiers.
tests are failing again on 0.7-beta. perhaps 0.7 should be added to travis.yml? |
Sure. |
The remaining failure is fixed with DataStreams master.