search

JuliaData / Feather.jl

Read and write feather files in pure Julia
https://juliadata.github.io/Feather.jl/stable
Other
109 stars 27 forks source link

Feather master breaks Requests.jl #83

Closed cstjean closed 6 years ago

cstjean commented 6 years ago

On 0.6, OSX

julia> using Feather, Requests
ERROR: InitError: ArgumentError: '/home/cst-jean/.julia/v0.6/MbedTLS/src/../deps/cacert.pem' is not in feather format: header = UInt8[0x23, 0x23, 0x0a, 0x23], footer = UInt8[0x2d, 0x2d, 0x2d, 0x0a].
Stacktrace:
 [1] validatefile(::String, ::Array{UInt8,1}) at /home/cst-jean/.julia/v0.6/Feather/src/loadfile.jl:11
 [2] #loadfile#3(::Bool, ::Function, ::String) at /home/cst-jean/.julia/v0.6/Feather/src/loadfile.jl:19
 [3] (::Feather.#kw##loadfile)(::Array{Any,1}, ::Feather.#loadfile, ::String) at ./<missing>:0
 [4] #Source#4 at /home/cst-jean/.julia/v0.6/Feather/src/source.jl:18 [inlined]
 [5] (::Core.#kw#Type)(::Array{Any,1}, ::Type{Feather.Source}, ::String) at ./<missing>:0
 [6] crt_parse_file at /home/cst-jean/.julia/v0.6/MbedTLS/src/x509_crt.jl:36 [inlined]
 [7] ca_chain!(::MbedTLS.SSLConfig) at /home/cst-jean/.julia/v0.6/MbedTLS/src/ssl.jl:81
 [8] get_default_tls_config(::Bool) at /home/cst-jean/.julia/v0.6/Requests/src/streaming.jl:101
 [9] __init_streaming__() at /home/cst-jean/.julia/v0.6/Requests/src/streaming.jl:222
 [10] __init__() at /home/cst-jean/.julia/v0.6/Requests/src/Requests.jl:34
 [11] _include_from_serialized(::String) at ./loading.jl:157
 [12] _require_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:200
 [13] _require_search_from_serialized(::Int64, ::Symbol, ::String, ::Bool) at ./loading.jl:236
 [14] _require(::Symbol) at ./loading.jl:441
 [15] require(::Symbol) at ./loading.jl:405
during initialization of module Requests

@ExpandingMan ?

quinnj commented 6 years ago

The no-no is on this line; we can't import read because we want to define our own Feather.read(::String) method.

ExpandingMan commented 6 years ago

Wow, that was an embarrassing screw-up on my part. I'll fix this by tonight.

Looks like one of the artifacts of my very early experimental work with this that survived my purges when it shouldn't have.

Kind of amazing that I haven't encountered this error myself yet.

quinnj commented 6 years ago

No worries; we're being a bit funny anyway by defining our own Feather.read method; I like it and prefer that the top-level API be Feather.read, but it just means that all Base uses of read in Feather have to be explicitly referenced Base.read.

quinnj commented 6 years ago

We'll want to not import write either. And as a style note, I don't like doing these kind of import statements at all; I prefer to explicitly annotate the actual method definition like Base.size(::MyType) = .... That way I don't have to remember to update/modify an import list and it makes it very clear at method definition site that a method is either extending a Base method or defining its own.

ExpandingMan commented 6 years ago

Yes, I agree the Base.func syntax is much better, I've recently come around to this way of thinking about it.

I'll make all the changes tonight. Eventually I'll probably change Arrow.jl to that style as well.

ExpandingMan commented 6 years ago

Should be fixed now on master.

ExpandingMan commented 6 years ago

Assuming this is fixed. Please re-open if you are still experiencing an issue.

cstjean commented 6 years ago

Any chance that Feather.jl could be tagged? I've been using master for a month, and it's working great.

ExpandingMan commented 6 years ago

That's great to hear. I've also been using master extensively since I rewrote it and indeed everything seems fine.

@quinnj indicated that there is missing DataStreams integration, but that's up to him as I'm not sure what remains to be done.

I have no objections to tagging now. I think that, at the very latest, we should tag very soon after the 0.7 release.