BUG: Allow IOErrors when attempting to retrieve default client encoding.
State: closed

- [X] closes #21552 

When using mod_wsgi, access to sys.stdout is restricted by default. To handle this case, catch a base exception instead of a more specific AttributeError during this process to include the thrown IOError by mod_wsgi.


Labels:
name: Bug
name: IO Data
description: IO issues that don't fit into a more specific label

Comments:
- Is this referencing a particular issue?
- Is there any other option besides catching base Exception (tend to avoid this if we can)
- Can you add tests?
* Is this referencing a particular issue?
    - I haven't raised an issue for this as it just came up this morning.
* Is there any other option besides catching base Exception (tend to avoid this if we can)
    - We could instead widen the exception clause to just AttributeError and IOError.
* Can you add tests?
    - Will do.


OK thanks - if you could open an issue and reference it then that would be preferable (typically PRs are only allowed without an issue if they are DOC updates).

You'll also want a whatsnew note to describe the bug and reference the issue. Next minor release is 0.23.2 at this point
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@4e0b636`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21531/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##             master   #21531   +/-   ##
=========================================
  Coverage          ?   92.18%           
=========================================
  Files             ?      169           
  Lines             ?    50739           
  Branches          ?        0           
=========================================
  Hits              ?    46774           
  Misses            ?     3965           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.59% <100%> (?)` | |
| #single | `42.34% <0%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/formats/console.py](https://codecov.io/gh/pandas-dev/pandas/pull/21531/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvY29uc29sZS5weQ==) | `75.75% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=footer). Last update [4e0b636...81547da](https://codecov.io/gh/pandas-dev/pandas/pull/21531?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hello @JayOfferdahl! Thanks for updating the PR.





Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on August 23, 2018 at 03:18 Hours UTC
Thanks for the feedback, made some changes to tests to cover more cases and provide some insight as to what they're doing. Is there anything special I need to do to get this working with travis?
looks like it might be tricky to monkeypatch how you are doing it: https://travis-ci.org/pandas-dev/pandas/jobs/394566147

does this test run locally?
Interesting...it runs locally just fine...I'll probably need to patch stdout/stdin like the other tests and provide the property on that patched object.
I've updated the test to mock out stdout/stdin instead of trying to change their encoding attribute. I do the same in other tests so I'm assuming this will work fine with travis.

Apologies for the slow turn around on this one!
@WillAyd I've taken a look at the context manager and have updated these tests. Looks like I missed the 0.23.2 release...my own fault, of course. Should I put my note in the 0.24 whatsnew document? Not sure how you all handle bugs in non-minor release versions.

Side note: I did have to upgrade pytest to 3.6.3, I'm almost certain that this will break travis since it won't have that version (maybe I'm wrong) -- where does this get specified?
no problem bumping pytest version (separate PR)
Made the linter changes...Have there been any consideration into putting that step in a pre-commit hook? Other than that, it appears that the 0.23.3 release already went out (July 7th) so I put the line in the 0.23.4 whatsnew file; let me know if this was an error.
Thanks for the feedback on the mock_raises_exception. Using pytest.raises is much simpler. I moved the whatsnew entry to 0.24 and took it out of both of the others.
Any update here?
Well this is embarrassing: No. When I updated the tests to use pytest.raises in the parameterized sections, I failed to include a case where IOError was raised. The pytest.raises RaisesContext object was then sent into the first try block and raised it's own AttributeError (which was caught) because it doesn't have an encoding attribute. 

The reason I originally had the 'mock_raises_exception' madness was because I want the input to raise an exception when accessed, so that it may be caught inside the function. As far as I can tell, pytest doesn't offer this functionality (using Mock, I would just have this as a side_effect on my mocked object).

Am I missing something with pytest.raises? I'm not trying to assert that an exception being raised was the end of the test, but rather that the function handled the exception and moved on.
`pytest.raises` is typically used as a context manager so I don't think it serves the purpose for what you are trying to do. Does raising via a UDF not work?
Not from what I can see. This is bad news bears: I need to rethink these tests. The parameterization I've set up is throwing AttributeErrors for the wrong reasons (in the context of the test). 

Not sure how I would use a UDF for this either since the attribute I'm targeting isn't ever invoked. I've tried to setup a test class which takes an encoding and has an encoding property on it in which I raise the exception, but haven't found a clean way to do that yet.
I pushed an attempt at solving this problem. I ended up moving that MockEncoding class outside of the test class and extended it a little bit. Yes, I know this is a dirty solution -- let me know if you guys can think of any way to improve it.
Hmm yea this is kind of a corner case...would be nice with `mock` but I don't think that's feasible to manage with our Py27 compat. Unless @jreback has any objection I think we can stick with what you have, though add a comment to the class that says `TODO(py27): replace with mock` so we can get rid of it when we drop support
Sounds good to me. I did get a pylint warning about the TODO -- is this going to fail in Travis?
Looks like it. You can move the comment inline with the class name
Inline with the class name gave the same error...ended up putting this in the docstring. This should still be just as easy to discover, I assume.
Changes made. I was using the regular pylint package and not flake8, which explains the difference. Apologies, and thanks for spending so much time on this, I really appreciate it.
@JayOfferdahl can you rebase?
Of course, is the general work flow to just squash all of my changes down into one commit and rebase on top of master or should I merge master into this?
Merge master in
Any update here?
this is fine. can you rebase, and ping on green (@WillAyd  / @gfyoung ok to merge then)
Thanks @JayOfferdahl !
Thank you, and thanks for the help throughout the process!