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

0.7 updates #1320

Merged
merged 4 commits into from
Dec 19, 2017
Merged

0.7 updates #1320

merged 4 commits into from
Dec 19, 2017

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Dec 17, 2017

No description provided.

@@ -1,6 +1,10 @@
__precompile__(true)
module DataFrames

if !isdefined(Base, :pairs)
pairs(x) = x
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a dangerous definition to have lying around, but I guess it's okay until we stop supporting 0.6.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Shouldn't Compat provide something like that, since many packages will have the same problem?

BTW, is Femtocleaner able to remove this kind of isdefined check? Or should we use VERSION checks instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about FemtoCleaner, but pairs will be available in Compat as of 0.40.0: JuliaLang/METADATA.jl#12530

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do we want to use Compat.pairs here rather than a custom method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say so, now that it's available in Compat. Just have to bump the REQUIRE to 0.40.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now we might as well wait till Compat gets Unicode since DataFrames show totally breaks w/o it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, I'd say just add

if VERSION >= v"0.7.0-DEV.2915"
    using Unicode
end

@ararslan
Copy link
Member

WOW that 0.7 failure. 😱 Reported at JuliaLang/julia#25176

@quinnj quinnj merged commit 855c963 into master Dec 19, 2017
@quinnj quinnj deleted the jq/pairs branch December 19, 2017 12:55
@bkamins
Copy link
Member

bkamins commented Dec 22, 2017

I believe that this line:
cbb54c7#diff-0aa1ff52d56badabf7d1511d3dc3f279R119
is not working correctly with Julia 0.6 and all DataFrames tests will fail:

julia> using Compat.pairs

julia> function test(; kwargs...)
           collect((k => v for (k,v) in pairs(kwargs)))
       end
test (generic function with 1 method)

julia> test(a=1,b=2)
2-element Array{Pair{Int64,Tuple{Symbol,Int64}},1}:
 1=>(:a, 1)
 2=>(:b, 2)

or I am missing something?

@nalimilan
Copy link
Member

Did I approve the changes despite CI not passing? I'm too used to seeing 0.7 fail, I didn't even check whether 0.6 passed...

@ararslan
Copy link
Member

Yeah 0.6 was failing; I was surprised this got merged as-is.

@nalimilan
Copy link
Member

See #1329.

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

Successfully merging this pull request may close these issues.

4 participants