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

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

Closed
iwelch opened this issue Oct 1, 2018 · 6 comments
Closed

Comments

@iwelch
Copy link

iwelch commented Oct 1, 2018

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
Copy link
Collaborator

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
Copy link
Author

iwelch commented Oct 4, 2018

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

@ExpandingMan
Copy link
Collaborator

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

@cstjean
Copy link
Contributor

cstjean commented Nov 7, 2018

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

@ExpandingMan
Copy link
Collaborator

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
Copy link
Collaborator

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.

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

No branches or pull requests

3 participants