Skip to content

BUG: Check for null values when infering excel in read_clipboard #41109

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
Merged
merged 9 commits into from
May 26, 2021
Merged

BUG: Check for null values when infering excel in read_clipboard #41109

merged 9 commits into from
May 26, 2021

Conversation

saucoide
Copy link
Contributor

@saucoide saucoide commented Apr 22, 2021 •
edited
Loading

GH41108
Stripping whitespace on read_clipboard causes data copied from
excel to lose the tab separator between the first column and
the nexts, shifting the data into the wrong column

This just removes the .lstrip() except for the first line
containing headers, to avoid breaking header-less index in csv
files

@pep8speaks
Copy link

pep8speaks commented Apr 22, 2021 •
edited
Loading

Hello @saucoide! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-05-22 17:07:04 UTC

@saucoide saucoide mentioned this pull request Apr 23, 2021
Closed
3 tasks
@phofl
Copy link
Member

phofl commented Apr 23, 2021

Please add whatsnew and fix linting issue

rhshadrach
rhshadrach requested changes Apr 25, 2021
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Question on the fix.

@jreback jreback added IO Data IO issues that don't fit into a more specific label IO Excel read_excel, to_excel labels Apr 26, 2021
@saucoide saucoide requested a review from rhshadrach May 7, 2021 22:49
rhshadrach
rhshadrach requested changes May 8, 2021
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the update, I think the approach looks good. Some requests/comments

rhshadrach
rhshadrach requested changes May 15, 2021
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! I think looks really good - but I don't understand the use of replace in the test.

@rhshadrach
Copy link
Member

Thanks - looks good! Can you resolve the merge conflict.

saucoide added 9 commits May 22, 2021 19:00
@saucoide
BUG: Check for null values when infering excel in read_clipboard
0997bc7 
GH41108
Stripping whitespace on read_clipboard causes data copied from
excel to lose the tab separator between the first column and
the nexts, shifting the data into the wrong column

This just removes the .lstrip() except for the first line
containing headers, to avoid breaking header-less index in csv
files
@saucoide
separated test & added whatsnew
8310492
@saucoide
test compares the whole dataframe & reformatted
a08502d
@saucoide
change test to use tm.assert_frame_equal
d468588
@saucoide
Check for index columns and pass through the kwarg to read_csv
7438ab9 
Use the number of leading tab's in the first row when we are
copying from excel as the value for `index_col`
@saucoide
removing print
efab5a8
@saucoide
parametrized multiindex test & fixed the args to index_col
1603426
@saucoide
Remove .replace() and use implicit str concatenation instead
0b1e21a
@saucoide
make the concatenation explicit
030939e
@saucoide saucoide force-pushed the read_clipboard_nulls branch from 95e806b to 030939e Compare May 22, 2021 17:06
rhshadrach
rhshadrach approved these changes May 24, 2021
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm, @phofl - good here?

@simonjayhawkins simonjayhawkins added this to the 1.3 milestone May 25, 2021
@jreback jreback merged commit 2f7be5e into pandas-dev:master May 26, 2021
@jreback
Copy link
Contributor

jreback commented May 26, 2021

thanks @saucoide

TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
@saucoide @TLouf
BUG: Check for null values when infering excel in read_clipboard (pan…
5d81894 
…das-dev#41109)
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
@saucoide @JulianWgs
BUG: Check for null values when infering excel in read_clipboard (pan…
f007bd9 
…das-dev#41109)
@mzeitlin11 mzeitlin11 mentioned this pull request Sep 5, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phofl phofl phofl left review comments

@rhshadrach rhshadrach rhshadrach approved these changes

Assignees
No one assigned
Labels
Bug IO Data IO issues that don't fit into a more specific label IO Excel read_excel, to_excel
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

BUG: read_clipboard() on excel shifts values to the wrong column when there are nulls in the first column
6 participants
@saucoide @pep8speaks @phofl @rhshadrach @jreback @simonjayhawkins