Skip to content
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

Python34 compatibility #7

Merged
merged 3 commits into from
Feb 8, 2015
Merged

Python34 compatibility #7

merged 3 commits into from
Feb 8, 2015

Conversation

WnP
Copy link
Contributor

@WnP WnP commented Feb 5, 2015

Tested on python3.4 and python2.7.9

@lukasschwab
Copy link
Owner

@WnP thanks for the pull request!

I'm going to make some minor line comments in a moment but, as someone not super familiar with the changes between Python 2.7 and Python 3.3 // 3.4, I'd like a more detailed explanation of why these changes are necessary for forward-compatibility before I merge this pull request or evaluate it in detail.

Thanks!

@WnP
Copy link
Contributor Author

WnP commented Feb 5, 2015

for the first commit 88e2ecd this answer on stackoverflow should help you to understand the problem

for the second commit 1829ed8 this other answer on SO should help

@lukasschwab
Copy link
Owner

Re. commit 88e2ecd

@WnP I'm testing on python 2.7.6, and I'm getting an error:

  File "stackit/stackit_core.py", line 210, in <module>
    main()
  File "stackit/stackit_core.py", line 199, in main
    term = getTerm(parser)
  File "stackit/stackit_core.py", line 135, in getTerm
    term += (output.splitlines()[-1] + bytes(" ", "ascii"))
TypeError: str() takes at most 1 argument (2 given)

And accoding to this SO thread that makes sense––bytes functions like string and does not take encoding as an argument.

Maybe add a condition that checks the python version? I can't merge this as is.

@WnP
Copy link
Contributor Author

WnP commented Feb 7, 2015

@lukasschwab fixed

@lukasschwab
Copy link
Owner

Looks great, problem solved!
Merging + adding README credits

lukasschwab added a commit that referenced this pull request Feb 8, 2015
Python34 compatibility
@lukasschwab lukasschwab merged commit 40b8bea into lukasschwab:master Feb 8, 2015
lukasschwab added a commit that referenced this pull request Feb 8, 2015
@WnP WnP deleted the python34 branch February 8, 2015 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants