ENH: loosen XLS signature
State: closed

- [x] closes #41225
- [x] tests added / passed
- [x] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [x] whatsnew entry

Added ability to check for multiple XLS signatures according to testing files available at [Spreadsheet Project](https://www.openoffice.org/sc/testdocs/). Also defer raising an error when the engine is specified but the file signature does not match one of the values in `SIGNATURES`: this allows a user to attempt to specify and use an engine even if the passed file doesn't match one of the provided values in `SIGNATURES`.

This is my first PR for this project, so please let me know if more is expected (writing tests, writing a whatsnew entry, etc). I have run Flake8 on the code and successfully opened BIFF2 through BIFF8 files with this method. Thanks!


Labels:
name: Enhancement
name: IO Excel
description: read_excel, to_excel

Comments:
@phofl Thanks for the heads up. I didn't set up the pandas dev environment properly so I'm going to try to take care of that today and address the failing tests.
@phofl @rhshadrach  I think I've got it mostly squared away, however the Azure pipeline is throwing an error because it can't import `xlrd` (in order to catch `XLRDError`). Do you have a suggestion for how to resolve this? Thanks!
xlrd is not installed in every ci, you should probably import this in your test runtime
> xlrd is not installed in every ci, you should probably import this in your test runtime

Just to clarify, do you mean the import should be moved to inside the testing class method? Will the test be bypassed if xlrd is not installed in the ci?
Yes into the if engine == xlrd 
If you look at the beginning of the file you will See that the engines are constructed based on the installed packages 
@phofl I have updated the existing related tests, should I also write new ones that verify that `read_excel` will read older XLS files, or are we good to go?

Thanks for your help and patience!
Please add new tests, if the old tests don't cover everything. Since you've added a bunch of new signatures we need test for every one of them
> Thanks for the PR! Looks really good, some minor requests below. Agree with @phofl on the need for tests.

Thanks for the feedback! Hoping to add the tests today. I imagine it's considered bad form to write test that rely on downloading outside resources (specifically the various Excel files hosted at https://www.openoffice.org/sc/testdocs/). If that's the case, should the tests just use `io.BytesIO` objects that are created using the values in the `SIGNATURES` dictionary?
Yes, creating the tests without external files would be perfect
I added a test, but I'm afraid it's probably not the best. I haven't been able to find blank BIFF2, BIFF3, BIFF4, BIFF5 files in order to  observe their header and body bytes to make more accurate `io.BytesIO` streams to test against. I have examined files from the [Spreadsheet Project's Test Documents](https://www.openoffice.org/sc/testdocs/), but since those files are not empty (and I imagine some of the first few bytes describe how long the document is, etc) I'm not sure it's the best solution.

If `xlrd` throws the `struct.error` exception, then the file has passed `xlrd`'s type inspection and has been determined to be a valid BIFF file (otherwise it would throw a custom BIFF exception). I imagine the reason the `struct.error` gets thrown is because the length of the `BytesIO` does not match the length specified in the header/body bytes (although this is just speculation).

Any advice or suggestions would be greatly appreciated.
@phofl @rhshadrach I believe I have addressed all the concerns (please let me know if you're still expecting anything). Should I click the "re-request review" buttons for your two requests? Thanks!
Could you merge master? I'll leave the final go to @rhshadrach 
Thanks @geoffrey-eisenbarth!
@rhshadrach thanks for your guidance!