Skip to content

Ods loses spaces 32207#33233

Merged
jreback merged 7 commits into
pandas-dev:masterfrom
detrout:ods-loses-spaces-32207
Apr 6, 2020
Merged

Ods loses spaces 32207#33233
jreback merged 7 commits into
pandas-dev:masterfrom
detrout:ods-loses-spaces-32207

Conversation

@detrout

@detrout detrout commented Apr 2, 2020

Copy link
Copy Markdown
Contributor
  • closes read_excel loses spaces on ods #32207
  • tests added / passed tests.io.excel.test_reader_spaces
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff (I had to use git diff master though)
  • whatsnew entry. It's only a bug fix does it need a whatsnew?

I'm not sure how you make xlsxb files so that test wasn't implement when checking for the different types of space.

There's quite possibly many other ways this parser doesn't handle the odf specification.

@WillAyd WillAyd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just pushed an .xlsb copy up; did it with a "Save As" from microsoft excel not sure if supported by other tools

Comment thread pandas/io/excel/_odfreader.py
@WillAyd WillAyd added the IO Excel read_excel, to_excel label Apr 2, 2020
@WillAyd WillAyd added this to the 1.1 milestone Apr 2, 2020
@WillAyd

WillAyd commented Apr 2, 2020

Copy link
Copy Markdown
Member

Can you also add a whatsnew for 1.1?

@WillAyd

WillAyd commented Apr 2, 2020

Copy link
Copy Markdown
Member

Can you run black on the changes? Should fix the code checks failure

The linux 37 failure is probably unrelated

@detrout

detrout commented Apr 2, 2020

Copy link
Copy Markdown
Contributor Author

While I was filling out the form I ran black, but forgot to push the commit.

And thanks for uploading a xlsb file, I don't have a copy of excel here at home.

@WillAyd

WillAyd commented Apr 3, 2020

Copy link
Copy Markdown
Member

If you can add a whatsnew note for 1.1 this lgtm - thanks!

@detrout

detrout commented Apr 3, 2020

Copy link
Copy Markdown
Contributor Author

How's the description?

@WillAyd

WillAyd commented Apr 3, 2020

Copy link
Copy Markdown
Member

@jreback care to look?

@jreback

jreback commented Apr 3, 2020

Copy link
Copy Markdown
Contributor

looks ok, i imagine this would slow down writing a bit?

@WillAyd

WillAyd commented Apr 3, 2020

Copy link
Copy Markdown
Member

Hmm fair point. @detrout do you mind run asv continuous upstream/master -b io.excel.ReadExcel from the asv_bench directory and posting output here?

Yea in any case fixes a bug so probably not a blocker, but might want to be cognizant of perf impact(s) if any

@detrout

detrout commented Apr 3, 2020

Copy link
Copy Markdown
Contributor Author

looks ok, i imagine this would slow down writing a bit?

There's no OpenDocument write support so I don't think it can slow down writes.
I'll need to figure out how to have a stable test environment for running a benchmark

@jreback jreback merged commit f404a3f into pandas-dev:master Apr 6, 2020
@jreback

jreback commented Apr 6, 2020

Copy link
Copy Markdown
Contributor

thanks @detrout

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

Labels

IO Excel read_excel, to_excel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

read_excel loses spaces on ods

3 participants