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

Fix vcat in case no data frames are passed #3081

Merged
merged 7 commits into from
Jun 19, 2022
Merged

Fix vcat in case no data frames are passed #3081

merged 7 commits into from
Jun 19, 2022

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Jun 18, 2022

Fixes #3074

@bkamins bkamins requested a review from nalimilan June 18, 2022 08:20
@bkamins bkamins added the bug label Jun 18, 2022
@bkamins bkamins added this to the 1.4 milestone Jun 18, 2022
@nalimilan
Copy link
Member

I also noticed this weird behavior. AFAICT it's not addressed by this PR?

julia> vcat(DataFrame(), DataFrame())
0×0 DataFrame

julia> vcat(DataFrame(), DataFrame(), cols=[:a, :b])
0×0 DataFrame

julia> vcat(DataFrame(a=1:2), DataFrame(), cols=[:a, :b])
2×2 DataFrame
 Row │ a      b       
     │ Int64  Missing 
─────┼────────────────
   1 │     1  missing 
   2 │     2  missing

nalimilan
nalimilan reviewed Jun 18, 2022
View reviewed changes
test/dataframe.jl Outdated Show resolved Hide resolved
test/dataframe.jl Outdated Show resolved Hide resolved
bkamins and others added 2 commits June 18, 2022 21:04
@bkamins @nalimilan
Update test/dataframe.jl
026b271 
Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
@bkamins
improve tests
611b34f
@bkamins
Copy link
Member Author

bkamins commented Jun 18, 2022

AFAICT it's not addressed by this PR?

It is addressed by this PR. I have added tests showing this and a comment why it is covered (essentially the reason is that in _vcat the dfs argument has DataFrame() data frames filtered-out).

nalimilan
nalimilan approved these changes Jun 18, 2022
View reviewed changes
src/abstractdataframe/abstractdataframe.jl Outdated Show resolved Hide resolved
@bkamins bkamins merged commit d645276 into main Jun 19, 2022
@bkamins bkamins deleted the bk/vcat_fix branch June 19, 2022 06:24
@bkamins
Copy link
Member Author

bkamins commented Jun 19, 2022

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.

Fix handling of cols kwarg in vcat in case no data frames are passed
2 participants
@bkamins @nalimilan