ENH: XLSB support
State: closed

Hey all, a moderately commonly requested feature is xlsb support. I thought I'd go ahead and make a PR for it, based on Pyxlsb. The library isn't very full-featured: datetimes are loaded in as floats without any indication they're datetimes. Would that be grounds for rejection?

Alternative would be to implement xlsb support in Openpyxl which looks like it will take a long time for someone not familiar with the file formats (as I am).

- [X] closes #8540
- [x] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: IO Excel
description: read_excel, to_excel

Comments:
to get the CI to pass, can you run `isort pandas/io/excel/_pyxlsb.py`
@jbrockmendel thanks for the tip! Did it alphabetically but apparently there's some kind of convention.

The documentation not compiling looks like a fun bug. Will try and see if I can reproduce and squash it tomorrow.
> Did it alphabetically but apparently there's some kind of convention.

Yah, isort has its own conventions and then we have some configuration in setup.cfg, not sure what the problem was here, but looks resolved now, thanks.

> The documentation not compiling looks like a fun bug. Will try and see if I can reproduce and squash it tomorrow.

That'd be great, its affecting a lot of PRs right now
Haven't been able to reproduce the bug on my computer, but I did do the necessary modifications to the test suite. Currently it's failing a lot of tests because `ExcelFile` appears to get called with no engine and that raises an `XLRDError`. Any help here? I'm not sure `ExcelFile` should get called at all with no engine and an .xlsb-file, since I added the appropriate checks near the beginning of the fixture.

A bunch of tests are not passing because (seems like) `pd.ExcelFile` doesn't get patched to use the `pyxlsb` engine. Can anyone see what's going on with that, as I really don't know a whole lot about the way pytest works. I would anticipate the `cd_and_set_engine`-fixture to be used for every test in the class...
Found it, there was an addiional fixture I had missed `read_ext == ".xlsb" and engine != "pyxlsb"` line on. Thanks for the help as well, @WillAyd!

All of the failing tests look normal, having to do with not recognizing datetimes. There's two which have to do with a column header not being converted to "Unnamed: 0", but rather to "None". Probably something with types
Alright, seems like most things are in order now. Commits are a bit messy but I'm not great at git yet.

The one build that's failing is failing because jinja2 is doing weird stuff, not sure about that.
> Commits are a bit messy but I'm not great at git yet.

Dont worry about it, we squash everything on merge so it wont matter before long.  git is one of those learning curves that never seems to end.

> The one build that's failing is failing because jinja2 is doing weird stuff, not sure about that.

Unrelated
Can you update the extended summary for `read_excel`? It says that you can read `.xls` or `.xlsx` files - should update for `.xlsb` as well as `.odf` and `.xlsm`
Oh, right... pyxlsb is not available on conda. What's the proper way of solving this issue?
> Oh, right... pyxlsb is not available on conda. What's the proper way of solving this issue?

you add pip dependencies in the ci/deps, e.g. pandas-dev/ci/deps/travis-37.yaml
where we add moto
@jreback finally got around to fixing the builds. Checks are failing because tests are failing, which are exactly the features missing from pyxlsb we talked about above. Do I disable those tests for pyxlsb?
AFACIT pyxlsb doesn't abide by PEP 396 which makes version introspection more difficult. Are you aware of a runtime way to determine the version? If not somewhat hesitant on this without maybe that change being made upstream first as we use that for compat when dealing with almost all third party dependencies
@Rik-de-Kort can you merge master, ping on green,
Opened https://github.com/wwwiiilll/pyxlsb/issues/22 for the version issue.
Hi, I'm the pyxlsb dev, I'm glad this is getting incorporated here, I've had wwwiiilll/pyxlsb#12 open for a while for this integration specifically.

I've made the changes for PEP 396, so starting with 1.0.6 the version is available on the module.

I noticed that there's a TODO about buffer support on open, however the argument is passed straight to Python's `ZipFile` which will handle file-like things, so I think it should work as-is.

For better compatibility with future versions, I'd recommend using `.rows(sparse=False)` instead of iterating the sheet object directly to ensure consistent behavior when the default eventually changes for the faster sparse read.

I'll maintain backward compatibility with the 1-based `get_sheet` and instead introduce a new method with the proper behavior, this should avoid an eventual annoying compatibility issue.

Also, I'm aware of the whole date thing (wwwiiilll/pyxlsb#10). Adding the necessary style handling to detect date-like cells still require much work at the moment. This is currently my priority, whenever I can.
can u rebase and see if u can get passing
Pushing to 1.1
Hey, sorry for going AWOL, holidays and stuff. Will rebase tonight.
@jreback pinging on green (two weeks later lol)
@Rik-de-Kort need to pytest.xfail the tests with date times that aren't supported
Done. There's still a failing test (read from url), due to pulling the test file from the master branch of the repo, where the files won't be present till this pull request is complete. :)
Yea I've seen that network one before. Can you xfail here and then fix in a follow up PR after this gets merged?
> Yea I've seen that network one before. Can you xfail here and then fix in a follow up PR after this gets merged?

Yep, done in the latest commit. I added two xfails since the test is going to fail regardless even if test1.xlsb is present in the master branch.
@wwwiiilll just realized that because of the PEP 396 addition I might need the right version of Pyxlsb in the environment files. Currently it's 1.0.5, which doesn't have it. Will putting 1.0.6 suffice?
this is prob ok for 1.0.0, if you are ok with it @WillAyd 
Oh yea sure
@Rik-de-Kort very nice PR. If you can fix up comments on version and unskip the test that fails because it requires the file on master in a follow up would be much appreciated
Am I correct in assuming no further work is needed?
If you can clarify the min version required in a follow up (I think should be 1.0.6 not 1.0.5) that should be it