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

Handle xls record type BoolErr (number 517); fixes #259, relates to #… #274

merged 2 commits into from Feb 26, 2017


None yet
2 participants

jennybc commented Feb 26, 2017

…270, relates to #62

This was the last remaining xls cell type that wasn't handled. Incidentally, it's also the one used for Boolean cells, which we recently discussed, so xls is now as ready as xlsx to exploit CELL_LOGICAL, when it exists.

Record type 517 is also used for non-formula errors, such as explicit #N/A on the Excel side. I regard these cells as numeric if Boolean (in future: logical) and text otherwise (non-formula error). I will need to do more work re: cell reading to translate error codes into strings (whether explicit or arising from formula).

I've added some explicit Booleans and errors (formula and not) to the types.xls[x] testing sheet.

This also clears up the remaining fishy result when I do R -d valgrind -f testthat.R.

@jennybc jennybc requested a review from hadley Feb 26, 2017

Handle xls record type BoolErr (number 517); fixes #259, relates to #270
, relates to #62

This record type is also used for errors, but only errors that are not a result of a formula, which is most of them. So if OP on #62 provides an example, we could conceivably do more in the else branch here.

hadley approved these changes Feb 26, 2017

@jennybc jennybc merged commit aa95660 into tidyverse:master Feb 26, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
continuous-integration/travis-ci/pr The Travis CI build is in progress

@jennybc jennybc deleted the jennybc:xls-BoolErr branch Feb 26, 2017

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