Skip to content

ENH: add support for reading binary Excel files (#15914) #30519

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 7 commits into from
Jan 3, 2020
Merged

ENH: add support for reading binary Excel files (#15914) #30519

merged 7 commits into from
Jan 3, 2020

Conversation

ftruzzi
Copy link
Contributor

@ftruzzi ftruzzi commented Dec 27, 2019 •
edited
Loading

Hi, this is a quick fix for #15914.

After reading the issue I'm not sure if this is meant to be supported though, please let me know!

I ran into this issue when trying to pass to read_excel a binary file downloaded with requests so I think it would make sense to support this natively without the user having to figure out the need to call io.BytesIO first. If so, do we need to update other docstrings?

Thank you!

@gfyoung gfyoung added expressions pd.eval, query IO Excel read_excel, to_excel Enhancement and removed expressions pd.eval, query labels Dec 28, 2019
gfyoung
gfyoung requested changes Dec 28, 2019
View reviewed changes
Copy link
Member

@gfyoung gfyoung left a comment

Choose a reason for hiding this comment

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

This enhancement seems reasonable to me.

If so, do we need to update other docstrings?

Yes! Please update the docstring for read_excel as well as io.rst if necessary

cc @WillAyd

Also, welcome to pandas ! 🚀

@ftruzzi
Copy link
Contributor Author

ftruzzi commented Dec 29, 2019

Thank you @gfyoung, I have updated the read_excel docstring

@ftruzzi ftruzzi requested a review from gfyoung December 29, 2019 13:37
@WillAyd WillAyd added this to the 1.0 milestone Jan 3, 2020
WillAyd
WillAyd requested changes Jan 3, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

see question above. otherwise lgtm

gfyoung
gfyoung reviewed Jan 3, 2020
View reviewed changes
WillAyd
WillAyd approved these changes Jan 3, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Lgtm merge when happy @gfyoung

@gfyoung gfyoung merged commit 7da951f into pandas-dev:master Jan 3, 2020
@gfyoung
Copy link
Member

gfyoung commented Jan 3, 2020

Thanks @ftruzzi!

@ftruzzi
Copy link
Contributor Author

ftruzzi commented Jan 3, 2020

Thank you @gfyoung @simonjayhawkins @WillAyd for your help and thorough reviews! Happy to contribute :)

@ftruzzi ftruzzi deleted the excel_binary branch January 3, 2020 11:58
@mwtoews mwtoews mentioned this pull request Sep 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfyoung gfyoung gfyoung requested changes

@simonjayhawkins simonjayhawkins simonjayhawkins left review comments

@WillAyd WillAyd WillAyd approved these changes

Assignees
No one assigned
Labels
Enhancement IO Excel read_excel, to_excel
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

read_excel does not work on excel file binary text or buffered binary text object
4 participants
@ftruzzi @gfyoung @WillAyd @simonjayhawkins