-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
BUG: Allow IOErrors when attempting to retrieve default client encoding. #21531
Merged
WillAyd
merged 23 commits into
pandas-dev:master
from
JayOfferdahl:get-client-encoding-on-mod-wsgi
Sep 19, 2018
Merged
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
6c2987d
BUG: Except IOErrors when attempting to retrieve default client encod…
c31b914
BUG: Allow IOErrors when attempting to retrieve client encoding. (#21…
JayOfferdahl 510f4b1
Merge branch 'pending-branch' into get-client-encoding-on-mod-wsgi
JayOfferdahl 1afda5f
Merge branch 'master' into get-client-encoding-on-mod-wsgi
JayOfferdahl b37cb75
Fixup tests to add more scenarios. Code review changes.
JayOfferdahl 0b1ca1d
Merge branch 'get-client-encoding-on-mod-wsgi' of https://github.com/…
JayOfferdahl bf26adb
PEP8 changes.
JayOfferdahl 77a95d2
Mock out stdout/stdin instead of trying to access their properties.
JayOfferdahl 2598595
Merge branch 'master' into get-client-encoding-on-mod-wsgi
JayOfferdahl 59e0993
Use context manager when monkeypatching standard library items.
JayOfferdahl 8b51b34
Merge branch 'get-client-encoding-on-mod-wsgi' of https://github.com/…
JayOfferdahl 984da5c
Merge branch 'master' into get-client-encoding-on-mod-wsgi
JayOfferdahl 4790e10
Fix linter erros/code review changes.
JayOfferdahl 6156825
Move fix to 0.24.0 release; simplify tests.
JayOfferdahl fb1dc2e
Attempt at trying to get side_effects right without using mock.
JayOfferdahl 7cedfdc
Add TODO(py27) to test helper.
JayOfferdahl 35c0b3b
Fix pylint error.
JayOfferdahl 49ee7b5
Fix silly linting errors. Use pep8.
JayOfferdahl 40b3588
Move raise_or_return to MockEncoding; clean up parameters.
JayOfferdahl 5e3c8af
Merge branch 'master' into get-client-encoding-on-mod-wsgi
JayOfferdahl f485223
Merge branch 'master' into get-client-encoding-on-mod-wsgi
JayOfferdahl ab67024
Merge remote-tracking branch 'upstream/master' into get-client-encodi…
JayOfferdahl 81547da
Merge branch 'get-client-encoding-on-mod-wsgi' of https://github.com/…
JayOfferdahl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
import pytest | ||
|
||
from pandas.io.formats.console import detect_console_encoding | ||
|
||
|
||
class MockEncoding(object): # TODO(py27): replace with mock | ||
""" | ||
Used to add a side effect when accessing the 'encoding' property. If the | ||
side effect is a str in nature, the value will be returned. Otherwise, the | ||
side effect should be an exception that will be raised. | ||
""" | ||
def __init__(self, encoding): | ||
super(MockEncoding, self).__init__() | ||
self.val = encoding | ||
|
||
@property | ||
def encoding(self): | ||
return self.raise_or_return(self.val) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current LINT error is because you are missing a blank line here |
||
@staticmethod | ||
def raise_or_return(val): | ||
if isinstance(val, str): | ||
return val | ||
else: | ||
raise val | ||
|
||
|
||
@pytest.mark.parametrize('empty,filled', [ | ||
['stdin', 'stdout'], | ||
['stdout', 'stdin'] | ||
]) | ||
def test_detect_console_encoding_from_stdout_stdin(monkeypatch, empty, filled): | ||
# Ensures that when sys.stdout.encoding or sys.stdin.encoding is used when | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can add a reference to the issue for this and the other new tests, i.e. |
||
# they have values filled. | ||
# GH 21552 | ||
with monkeypatch.context() as context: | ||
context.setattr('sys.{}'.format(empty), MockEncoding('')) | ||
context.setattr('sys.{}'.format(filled), MockEncoding(filled)) | ||
assert detect_console_encoding() == filled | ||
|
||
|
||
@pytest.mark.parametrize('encoding', [ | ||
AttributeError, | ||
IOError, | ||
'ascii' | ||
]) | ||
def test_detect_console_encoding_fallback_to_locale(monkeypatch, encoding): | ||
# GH 21552 | ||
with monkeypatch.context() as context: | ||
context.setattr('locale.getpreferredencoding', lambda: 'foo') | ||
context.setattr('sys.stdout', MockEncoding(encoding)) | ||
assert detect_console_encoding() == 'foo' | ||
|
||
|
||
@pytest.mark.parametrize('std,locale', [ | ||
['ascii', 'ascii'], | ||
['ascii', Exception], | ||
[AttributeError, 'ascii'], | ||
[AttributeError, Exception], | ||
[IOError, 'ascii'], | ||
[IOError, Exception] | ||
]) | ||
def test_detect_console_encoding_fallback_to_default(monkeypatch, std, locale): | ||
# When both the stdout/stdin encoding and locale preferred encoding checks | ||
# fail (or return 'ascii', we should default to the sys default encoding. | ||
# GH 21552 | ||
with monkeypatch.context() as context: | ||
context.setattr( | ||
'locale.getpreferredencoding', | ||
lambda: MockEncoding.raise_or_return(locale) | ||
) | ||
context.setattr('sys.stdout', MockEncoding(std)) | ||
context.setattr('sys.getdefaultencoding', lambda: 'sysDefaultEncoding') | ||
assert detect_console_encoding() == 'sysDefaultEncoding' |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move to 0.23.5? @jreback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a regression? If not, I would probably stay with
0.24.0
unless there is a compelling reason to backport-patch this bug.