Skip to content

TST: Document and test na_filter in read_excel #29171

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

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Oct 22, 2019

Closes #25453

Picks up where #26353 left off

@gfyoung gfyoung added Testing pandas testing functions or related to the test suite Docs IO Excel read_excel, to_excel labels Oct 22, 2019
@gfyoung gfyoung added this to the 1.0 milestone Oct 22, 2019
@gfyoung gfyoung force-pushed the excel-na-filter-support branch 2 times, most recently from 57af01a to 5ba0374 Compare October 22, 2019 20:30
@gfyoung gfyoung force-pushed the excel-na-filter-support branch from 5ba0374 to 435f73f Compare October 23, 2019 00:16

with pd.ExcelFile("test5" + read_ext) as excel:
parsed = pd.read_excel(
excel, "Sheet1", keep_default_na=True, na_values=["apple"], **kwargs
Copy link
Member

Choose a reason for hiding this comment

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

Looks good. Do we already have coverage for the keep_default_na=False case documented above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Which case are you talking about? The documentation mentions multiple keep_default_na=False cases.

Copy link
Member

Choose a reason for hiding this comment

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

Right was asking if we already have coverage on keep_default_na=False since mentioned in docs, but only True here (in contrast to na_filter)

Copy link
Member Author

@gfyoung gfyoung Oct 23, 2019

Choose a reason for hiding this comment

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

The test above the one I've added has coverage for keep_default_na=False.

@WillAyd WillAyd merged commit bc020f6 into pandas-dev:master Oct 23, 2019
@WillAyd
Copy link
Member

WillAyd commented Oct 23, 2019

Thanks @gfyoung

@gfyoung gfyoung deleted the excel-na-filter-support branch October 23, 2019 20:15
HawkinsBA pushed a commit to HawkinsBA/pandas that referenced this pull request Oct 29, 2019
Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
bongolegend pushed a commit to bongolegend/pandas that referenced this pull request Jan 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs IO Excel read_excel, to_excel Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

read_excel na_filter undocumented?
3 participants