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

can now read or write to general IO rather than just the filesystem #115

Closed ExpandingMan closed 5 years ago

ExpandingMan commented 5 years ago

Fixes #113. This is especially useful for doing things like reading and writing to Amazon S3 or anything else over a network (previously one would have had to write to disk first).

ExpandingMan commented 5 years ago

@quinnj , tests are failing because in the docker container Python segfaults when reading test2.feather in the docker container. However, I read the same file in Python locally and it works perfectly fine. Is it possible that the Python feather package or something else in the container needs updating? I can reproduce the segfault within the container, but only within the container. Anywhere else I read the test2.feather file in either Julia or Python it works perfectly fine.

ExpandingMan commented 5 years ago

Much weirder: the test passes with no issues on Julia nightly on both linux and mac. That makes absolutely no sense to me whatsoever. Any idea what might be going on with that container?

ExpandingMan commented 5 years ago

This package is basically unusable for me now because it doesn't have this.

Anybody have any ideas what's going on in the docker? Is there anyway to trigger this again? I'm totally and completely stumped I was never able to reproduce the error anywhere.

codecov-io commented 5 years ago

Codecov Report

Merging #115 into master will increase coverage by 10.1%. The diff coverage is 82.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   67.33%   77.43%   +10.1%     
==========================================
  Files           4        5       +1     
  Lines         150      164      +14     
==========================================
+ Hits          101      127      +26     
+ Misses         49       37      -12
Impacted Files Coverage Δ
src/Feather.jl 100% <ø> (ø)
src/sink.jl 91.66% <100%> (+21.39%) :arrow_up:
src/metadata.jl 94.11% <100%> (-2.66%) :arrow_down:
src/loaddata.jl 78.26% <57.14%> (ø)
src/source.jl 61.42% <66.66%> (+6.58%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d21118b...f32092e. Read the comment docs.

ExpandingMan commented 5 years ago

I'm in the process of updating this and also implementing better (but optional) round-trip python tests as discussed in #119.

ExpandingMan commented 5 years ago

Ok, I have removed all of the docker stuff completely. There is now an optional set of Python round-trip tests (the same as before). They can't run in travis because it will be missing dependencies, but anyone should have no trouble running them locally, one just needs pandas, and pyarrow. In the future if anyone can find a way to provision travis with this, those tests can be restored.

Note that the tests involving the Python byte array type have been removed. They were not working in master either. The reason this went unnoticed is because the error only seems to occur with updated pandas and pyarrow. We can fix this later (I still fully intend on finishing radically overhaulingg Arrow.jl, sorry I got stalled out for a while).

This should also work with the latest DataFrames.jl. I'd like to make another PR later to remove the DataFrames dependence entirely in favor of the Tables.jl interface only, but of course these will be breaking changes.

If anyone has any issues, please say so, otherwise I will merge this tomorrow.

ExpandingMan commented 5 years ago

I'm going to merge this today. Ideally we should tag this today as well, any objections?

ExpandingMan commented 5 years ago

Does anyone know if we have to do anything new with Registrator, or can we just tag?

sglyon commented 5 years ago

I think tagging will not do anything.

I've had success:

JuliaRegistrator commented 5 years ago

Error while trying to register: Register Failed @sglyon, it looks like you are not a publicly listed member/owner in the parent organization (JuliaData). If you are a member/owner, you will need to change your membership to public. See GitHub Help

ExpandingMan commented 5 years ago

I took the liberty of bumping the version number to 0.5.2 in Project.toml by committing directly to master.

@quinnj you may need to call the registrator as I am not a member of JuliaData.

Is this seriously completely decoupled from tagging now? I don't think I like that.

KristofferC commented 5 years ago

Is this seriously completely decoupled from tagging now? I don't think I like that.

You can install tagbot that will tag automatically for you or you can paste the git command that Registrator gives you to tag yourself.

ExpandingMan commented 5 years ago

That seems reasonable.

Still seems that Quinn needs to do this.

quinnj commented 5 years ago

I kicked of JuliaRegistrator; I also made @ExpandingMan an admin on this repo; is that enough to allow him to call JuliaRegistrator? Anybody know?

ExpandingMan commented 5 years ago

According to the message that was given to @sglyon , I would need to be a member of JuliaData.

sglyon commented 5 years ago

I guess you’ll just have to develop more awesome features quickly so you can tag again and test it out ;)