[BUG]: Fix regression in read_table with delim_whitespace=True

[phofl]

• closes BUG: read_table raises ValueError when delim_whitespace is set to True #35958
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

We could write a private function which is called from read_csv and read_table with the appropriate default_set alternatively.

Edit: Black_pandas formats files not touched by myself. Was there an update or change of guidelines?

[phofl]

Test failure seems unrelated

[dsaxton]

@phofl Looks like a whatsnew conflict if you can merge master

[phofl]

@dsaxton done

[simonjayhawkins]

see #36381 (comment)

[phofl]

@simonjayhawkins

Yes, it is correct, that this does not raise an error. But this is not a regression. This combination does not raise an error on 1.0.5. That would be a regular bugfix.

Should I add this here or make a separate PR, which won't be backported to 1.1.3?

[tacaswell]

Following up on the comments it #36381 , I agree that the behavior still does not match the docstring. I would change the docstring to be "pass the default", not "pass nothing" which is both easier (no code changes!) and clearer (I can think of a defend-able position that '' and None are both "nothing").

[phofl]

That is probably a better solution. I would add this here if this is ok @simonjayhawkins ?

[simonjayhawkins]

@simonjayhawkins

Yes, it is correct, that this does not raise an error. But this is not a regression. This combination does not raise an error on 1.0.5. That would be a regular bugfix.

Should I add this here or make a separate PR, which won't be backported to 1.1.3?

fair point. can either fix here or raise an issue for this. Not raising also applies to read_csv, so if done here should probably include read_csv and would probably need a second release note.

I think checking if sep has been passed can be done fairly easily with lib.no_default. If done in a separate PR could also be backported as a bugfix.

[simonjayhawkins]

🤔 I've just re-read the docs If this option is set to True, nothing should be passed in for the delimiter parameter. so not raising is a bug. but otoh if it is not raising, and we start raising we would normally put something in the api breaking changes section of the release notes. since this is targeted for a patch release, we should probably not include anything considered an api breaking change.

so ignore my previous comments except about adding a comment to the code for the next reader.

[phofl]

Ok, will do that. I will open a issue about fixing this for 1.2?

[phofl]

@simonjayhawkins Done

[simonjayhawkins]

Thanks @phofl generally lgtm pending green

[phofl]

@simonjayhawkins Failure seems unrelated?

[simonjayhawkins]

restarted travis ci

[phofl]

That seems to be an issue with the Pipeline. It fails consistently through the PRs

[simonjayhawkins]

That seems to be an issue with the Pipeline. It fails consistently through the PRs

Thanks. It's was failing intermittently on master and constantly on 1.1.x. I see it's now constantly failing on master too.

[simonjayhawkins]

😕 merged master and still arm failures. restarted.

[phofl]

@simonjayhawkins do you have any idea what the underlying reason might be?

[simonjayhawkins]

it's timing out. happening on several PRs. no idea why.

[jreback]

this looks fine, just resolved conflicts.

cc @gfyoung ok here?

[simonjayhawkins]

Thanks @phofl

[simonjayhawkins]

@meeseeksdev backport 1.1.x