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

Fix BytesWarning in Mapper.generate() #56

Merged
merged 1 commit into from Feb 13, 2016

Conversation

Projects
None yet
2 participants
@vstinner
Contributor

vstinner commented Nov 27, 2015

When python3 is run with -bb, str(bytes) raises a BytesWarning. On
Python 3, Mapper.generate() gets such BytesWarning because
script_name type is str whereas cache_key type is byte.

On Python 3, generate() now encodes the script_name to UTF-8 and uses
bytes concatenation to fix this issue.

To test the patch, run tests using "python3 -bb". For example using my tox patch:

$ . .tox/py3/bin/activate
(py3)$ python -bb $(which nosetests) tests/
@bbangert

This comment has been minimized.

Show comment
Hide comment
@bbangert

bbangert Jan 13, 2016

Owner

If .travis.yml is updated so that the tests can verify this, I'll be happy to merge it.

Thanks!

Owner

bbangert commented Jan 13, 2016

If .travis.yml is updated so that the tests can verify this, I'll be happy to merge it.

Thanks!

Fix BytesWarning in Mapper.generate()
When python3 is run with -bb, str(bytes) raises a BytesWarning. On
Python 3, Mapper.generate() gets such BytesWarning because
script_name type is str whereas cache_key type is byte.

On Python 3, generate() now encodes the script_name to UTF-8 and uses
bytes concatenation to fix this issue.

.travis.yml: Run tests using "python -bb $(which nosetests)" to raise
BytesWarning exception on bytes vs Unicode issue.
@vstinner

This comment has been minimized.

Show comment
Hide comment
@vstinner

vstinner Feb 9, 2016

Contributor

If .travis.yml is updated so that the tests can verify this, I'll be happy to merge it.

Oh sorry, I missed your comment (or forgot to reply).

I don't know Travis, but I tried to hack .travis.yml to run tests with -bb option of Python.

If it's not possible to run all tests using -bb because .travis.yml is not configurable enough, I suggest you to test locally and merge the change anyway.

Contributor

vstinner commented Feb 9, 2016

If .travis.yml is updated so that the tests can verify this, I'll be happy to merge it.

Oh sorry, I missed your comment (or forgot to reply).

I don't know Travis, but I tried to hack .travis.yml to run tests with -bb option of Python.

If it's not possible to run all tests using -bb because .travis.yml is not configurable enough, I suggest you to test locally and merge the change anyway.

@bbangert

This comment has been minimized.

Show comment
Hide comment
@bbangert

bbangert Feb 13, 2016

Owner

Thanks!

Owner

bbangert commented Feb 13, 2016

Thanks!

bbangert added a commit that referenced this pull request Feb 13, 2016

Merge pull request #56 from haypo/mapper_py3
Fix BytesWarning in Mapper.generate()

@bbangert bbangert merged commit 214d020 into bbangert:master Feb 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@vstinner vstinner referenced this pull request Feb 15, 2016

Merged

Add tox.ini #55

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment