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

Improve Error Message When Data Type is Not Supported (Or Convert) #98

Closed iwelch closed 6 years ago

iwelch commented 6 years ago

I just had a weird error

julia> Feather.write(filename, df)
ERROR: KeyError: key Char not found

eventually, it dawned on me that my data frame had a char column, and that this was the problem.

so, I would suggest

  1. character to string auto-conversion (and other suitable auto-conversion)
  2. a message like "column x (named 'n') is type T, which is not supported by Feather"

I hope that both are easy fixes.

ExpandingMan commented 6 years ago

It might be nice to have some auto-conversions, though I'm not sure how I feel about how permissive it should be. My first instinct would be to allow only auto-conversions where convert works without error, although even in that case it would have to be a small subset, because, for example convert(Int, 1.0) obviously works.

Where this gets hairy is that, in your example, your Chars would get converted to UInt8, not String. So, perhaps it would be best not to have any auto-conversions at all.

We definitely should improve the error message. Of course this is not too difficult, but I have been considering doing a more comprehensive rewrite of some of the componenets that might make this even easier. The problem the package has right now is that the metadata writing process is weirdly separate from the process of actually writing the columns. This is due in large part to how the format works (the metadata is all stored in a separate flat buffer), but I feel it should be a bit more unified in the source code, see discussion in #86.

iwelch commented 6 years ago

hi EM. just wanted to suggest little improvements as I am playing around with it. feel free to close. best, /iaw

ExpandingMan commented 6 years ago

At a minimum at some point we will improve the error messages, so I won't close this issue until that happens.

cstjean commented 6 years ago

Feather does not support storing Rationals, like 1//2. In that case, a conversion to Float64 would have been nice.

ExpandingMan commented 6 years ago

Some conversions would definitely be nice (we already have TimeType conversions), but we should proceed carefully. For example, I don't think that Rational should be automatically converted, as rationals are exact while floating point is not.

ExpandingMan commented 6 years ago

I'm going to close this issue since it's specifically about an error message that has now been implemented. Feel free to open new issues about conversions.