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

Improve error message when pushing/appending with promote=true #3356

Merged
merged 6 commits into from
Jul 15, 2023
Merged

Improve error message when pushing/appending with promote=true #3356

merged 6 commits into from
Jul 15, 2023

Conversation

bkamins
Copy link
Member

@bkamins bkamins commented Jul 10, 2023

Improve error in cases like:

append!(DataFrame(x=PooledArray(1:255, UInt8)), DataFrame(x=PooledArray(256:500, UInt8)); promote=true)

as reported by @jariji

@bkamins bkamins added the non-breaking The proposed change is not breaking label Jul 10, 2023
@bkamins bkamins modified the milestones: 1.7, patch Jul 10, 2023
@bkamins bkamins marked this pull request as ready for review July 13, 2023 12:02
@bkamins bkamins requested a review from nalimilan July 13, 2023 12:03
nalimilan
nalimilan approved these changes Jul 13, 2023
View reviewed changes
test/insertion.jl Outdated Show resolved Hide resolved
test/insertion.jl Outdated Show resolved Hide resolved
src/dataframe/insertion.jl Outdated
Comment on lines 304 to 306
"Maybe it was forgotten to require column " *
"element type promotion, which can be done " *
"by passing the promote=true keyword argument."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're at it, maybe it would be enough to say "Maybe you forgot passing promote=true?"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK - changed.

bkamins and others added 2 commits July 14, 2023 09:34
@bkamins @nalimilan
Apply suggestions from code review
c97fc00 
Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
@bkamins
simplify error message
773fb3c
@bkamins bkamins merged commit 453b446 into main Jul 15, 2023
6 of 7 checks passed
@bkamins bkamins deleted the bk/improve_push_error_message branch July 15, 2023 07:13
@bkamins
Copy link
Member Author

bkamins commented Jul 15, 2023

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
non-breaking The proposed change is not breaking
Projects
None yet
Milestone
patch
Development

Successfully merging this pull request may close these issues.
2 participants
@bkamins @nalimilan