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.

Sign up for GitHub

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

Jump to bottom

Add check if input is a table before trying alternative constructors #2341

Merged
merged 4 commits into from
Aug 2, 2020
Merged

Add check if input is a table before trying alternative constructors #2341

merged 4 commits into from
Aug 2, 2020

Commits on Aug 1, 2020

  1. Add check if input is a table before trying alternative constructors 

    Fixes JuliaData/CSV.jl#702. A DataFrame
constructor was removed from CSV.jl in its latest release (0.7.6) as
part of new deprecations for decoupling the two packages. What I forgot
was that we were relying on that constructor because of an ambiguity in
the generic DataFrame constructor fallback that mostly uses Tables.jl to
try and turn anything into a DataFrame. The problem is "mostly"; one of
the checks is if the input is an `AbstractVector` of `AbstractVectors`,
which it turns out `CSV.File` is! This is because it's considered an
`AbstractVector` of `Row`s, which themselves are `<: AbstractVector`. So
we were trying to basically take each row of a `CSV.File` and treat each
row as a column in that fallback constructor.

While it was proposed to put the DataFrame constructor back in CSV.jl
for now; that's pretty unsatisfying because we know we're going to
remove the DataFrames deprecation at some point, and when we do, we
want `DataFrame(CSV.File())` to work out of the box.

The fix here is just adding an additional check up front in the fallback
constructor if we already know the input is `Tables.istable(x)`. If so,
we'll avoid these extra corner case checks and just let the Tables.jl
machinery do the work.
    @quinnj
    quinnj committed Aug 1, 2020
    Configuration menu
    Copy the full SHA
    2bc9cfb View commit details
    Browse the repository at this point in the history

  2. Add a test

    @quinnj
    quinnj committed Aug 1, 2020
    Configuration menu
    Copy the full SHA
    c850d68 View commit details
    Browse the repository at this point in the history

  3. Add some more tests

    @bkamins
    bkamins committed Aug 1, 2020
    Configuration menu
    Copy the full SHA
    73ac4a0 View commit details
    Browse the repository at this point in the history

  4. Fix typo

    @bkamins
    bkamins committed Aug 1, 2020
    Configuration menu
    Copy the full SHA
    ae73a69 View commit details
    Browse the repository at this point in the history