Skip to content

Py3k compatibility #142

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

Closed
wants to merge 27 commits into from
Closed

Py3k compatibility #142

wants to merge 27 commits into from

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 5, 2013

All tests pass under tox against Python 2.6, 2.7, 3.2, and 3.3.

tseaver added 27 commits May 28, 2013 15:19
- Add Trove classifiers for supported Python versions.

- Test supported Python versions using tox.
The exception handling syntax problem is too hard.
- Sort per PEP8.

- Remove Python 2.5 workaround.

- Move sys.path munging to the 'if __name__ == __main__' stanza.
Also, drop use of 'iteritems'.
In Python2, the 'bytes()' builtin doesn't take an encoding argument.
Tests all now pass on Py3k, with two remaining failures on Python2.
@zagy
Copy link

zagy commented Feb 28, 2014

Hi, how is that with merging and releasing for python3 compatibility? I'd need it. Anything I can do to help?

@zagy
Copy link

zagy commented Feb 28, 2014

For the time being I've added a release of Tres' branch to http://download.gocept.com/packages/oauth2-1.5.211py3-1.zip

@coilysiren
Copy link

+1

@joestump
Copy link
Owner

See #158 as well. cc @jaitaiwan

@joestump joestump added the PY3K label Jul 29, 2015
@joestump
Copy link
Owner

@tseaver You interested in becoming a collaborator on this? Second solid PR I've seen from you so far. 👍

@jaitaiwan
Copy link
Contributor

@joestump which PR should we use?

@jaitaiwan
Copy link
Contributor

This one has collisions...

@joestump
Copy link
Owner

@jaitaiwan Up to you. I think yours is newer and had more eyes on it, which always gets my vote. I mainly wanted to ask if @tseaver was interested in helping out. 😆

@tseaver
Copy link
Contributor Author

tseaver commented Jul 29, 2015

I had just about enough mental swap space left to fix #141. I'm afraid that updating this one requires swapping way more of the code back into my head than I have available for the foreseeable future.

@joestump
Copy link
Owner

@tseaver haha, totally understand. Thanks so much for your help with #141 and your PRs. 🎉

@hayd
Copy link

hayd commented Jul 31, 2015

There's a subtley here IMO, should all the things return as utf8 encoded bytes (as is the idea in this PR) or unicode (as most of stdlib does).

In a trial merge I changed it to unicode and basically the only fail was test_to_url_nonascii. Thoughts?

@rickhanlonii
Copy link
Collaborator

@jaitaiwan @joestump I spent some time today trying to fix the tests on #176 and found that we're going to need to make rather extensive changes if we want to use that PR (i.e. in Python 3 urlparse.urlsplit returns bytes so string checks fail). Those changes seem to already be contained in this PR.

I'm thinking we get this merged into feat-python-3 so we can all work on getting python 3 compatibility working with this as a base.

@rickhanlonii
Copy link
Collaborator

I attempted a master on merge here.

All tests pass for 2.6 and 2.7. There are a only 2 of the new tests failing in python 3:

Failure
Traceback (most recent call last):
  File "/Users/euler/dev/python-oauth2/py3.4/lib/python3.4/site-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/Users/euler/dev/python-oauth2/tests/test_oauth.py", line 908, in test_request_nonutf8_bytes
    self.assertRaises(TypeError, oauth.Request, method="GET", url=url, parameters=params)
AssertionError: TypeError not raised by Request


Failure
Expected :'http[38 chars]asdf&foo=baz&uni_utf8=%C2%AE&bar=foo&multi=%5B[81 chars]3%98'
Actual   :'http[38 chars]asdf&uni_unicode_2=%C3%A5%C3%85%C3%B8%C3%98&un[79 chars]=baz'
 <Click to see difference>

Traceback (most recent call last):
  File "/Users/euler/dev/python-oauth2/tests/test_oauth.py", line 478, in test_to_url_works_with_non_ascii_parameters
    'http://example.com?oauth_consumer=asdfasdfasdf&'
AssertionError: 'http[38 chars]asdf&foo=baz&uni_utf8=%C2%AE&bar=foo&multi=%5B[81 chars]3%98' != 'http[38 chars]asdf&uni_unicode_2=%C3%A5%C3%85%C3%B8%C3%98&un[79 chars]=baz'
- http://example.com?oauth_consumer=asdfasdfasdf&foo=baz&uni_utf8=%C2%AE&bar=foo&multi=%5Bb%27FOO%27%2C+b%27BAR%27%5D&uni_unicode=%C2%AE&uni_unicode_2=%C3%A5%C3%85%C3%B8%C3%98
+ http://example.com?oauth_consumer=asdfasdfasdf&uni_unicode_2=%C3%A5%C3%85%C3%B8%C3%98&uni_utf8=%C2%AE&multi=%5B%27FOO%27%2C+%27BAR%27%5D&uni_unicode=%C2%AE&bar=foo&foo=baz

@jaitaiwan
Copy link
Contributor

Getting closer!

@hayd
Copy link

hayd commented Jul 31, 2015

for the latter test: %5Bb%27FOO%27%2C+b%27BAR%27%5D has the b in b'..' which I don't think it should. I think it's easiest just to split i on ? then & (and test the set).

Thoughts on the bytes vs unicode issue?

@jaitaiwan
Copy link
Contributor

I personally opt for unicode, I feel like its the way of the future...

@hayd
Copy link

hayd commented Jul 31, 2015

In which case (I have a merge) which fails only: test_to_url_nonascii (py2.6+ and py3) and test_to_url_works_with_non_ascii_parameters (py2). These are kinda tricky since the unicode arguments aren't valid unicode (so it feels like this is expected).

Edit: See test failures here: https://travis-ci.org/hayd/python-oauth2

@jaitaiwan
Copy link
Contributor

Hey @hayd do you reckon you'd be interested in becoming a contributor? Seems like you're super keen to help this project along! cc @joestump

@rickhanlonii
Copy link
Collaborator

+1

@jaitaiwan
Copy link
Contributor

cc @joestump: this is the PR we're looking at for py3k over my stuff.

@rickhanlonii
Copy link
Collaborator

@jaitaiwan @joestump should this be against develop or a feature branch?

@jaitaiwan
Copy link
Contributor

It's a big change so I'd say feature branch

On Mon, 3 Aug 2015 11:27 Rick Hanlon II [email protected] wrote:

@jaitaiwan https://github.com/jaitaiwan @joestump
https://github.com/joestump should this be against develop or a feature
branch?


Reply to this email directly or view it on GitHub
#142 (comment)
.

@rickhanlonii
Copy link
Collaborator

@tseaver can you re-open against branch feat-python-3?

@tseaver
Copy link
Contributor Author

tseaver commented Aug 3, 2015

@rickhanlonii I'm happy to have somebody rebase / merge my work to such a branch, but I'm afraid I don't have capacity for that effort at this point.

@rickhanlonii
Copy link
Collaborator

No sweat. @hayd and I both took a stab at it already, just wanted to give you the opportunity first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.