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

Accept XLSX files that do not contain a "styles" part #505

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@j6t
Copy link

j6t commented Sep 12, 2018

An XLSX file that does not contain a "styles" part triggers this error:

read_excel("foo.xlsx", sheet="Sheet1")
Error in sheets_fun(path) :
Evaluation error: Couldn't find '' in 'foo.xlsx'.

To fix this, mimic the check that cacheStringTable() performs also in
cacheDateFormats().

@jennybc

This comment has been minimized.

Copy link
Member

jennybc commented Sep 12, 2018

Can you open an issue with a bit more background and an example of a file with this quality? I don't believe I've ever seen this phenomenon. We'd also need related tests.

Johannes Sixt
Accept XLSX files that do not contain a "styles" part; fixes #506
An XLSX file that does not contain a "styles" part triggers this error:

  > read_excel("foo.xlsx", sheet="Sheet1")
  Error in sheets_fun(path) :
    Evaluation error: Couldn't find '' in 'foo.xlsx'.

To fix this, check for the presence of the part in cacheDateFormats() in
the same way that cacheStringTable() does.

@j6t j6t force-pushed the j6t:master branch from 7c8a6e9 to d15db7e Sep 13, 2018

@j6t

This comment has been minimized.

Copy link

j6t commented Sep 13, 2018

Thank you for your feedback. I've submitted issue #506 and added a test case to my proposed fix.

@j6t

This comment has been minimized.

Copy link

j6t commented Nov 3, 2018

Is there anything I can do to move this issue forward?

@jennybc

This comment has been minimized.

Copy link
Member

jennybc commented Nov 5, 2018

We work on things in cycles and readxl is not in such a phase right now. But I'll try to take a look to get you unstuck.

jennybc added a commit that referenced this pull request Dec 12, 2018

PR #505: accept .xlsx that lack a "styles" part (#528)
* Accept XLSX files that do not contain a "styles" part; fixes #506

An XLSX file that does not contain a "styles" part triggers this error:

  > read_excel("foo.xlsx", sheet="Sheet1")
  Error in sheets_fun(path) :
    Evaluation error: Couldn't find '' in 'foo.xlsx'.

To fix this, check for the presence of the part in cacheDateFormats() in
the same way that cacheStringTable() does.

* Rename test sheet; code style

* Beef up the test a bit

* Add NEWS bullet
@jennybc

This comment has been minimized.

Copy link
Member

jennybc commented Dec 12, 2018

Thanks!

I made a few small changes. Usually I can push back into someone's PR but that did not work here. So I continued in an internal branch and merged. You are credited in the NEWS.

#528

@jennybc jennybc closed this Dec 12, 2018

@jennybc

This comment has been minimized.

Copy link
Member

jennybc commented Dec 12, 2018

In future and in general, it is best if you make a pull request from a non-master branch in your fork.

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