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

Port to Python 3.x #5

Merged
merged 2 commits into from
Feb 6, 2017
Merged

Port to Python 3.x #5

merged 2 commits into from
Feb 6, 2017

Conversation

theFork
Copy link
Contributor

@theFork theFork commented Feb 5, 2017

I think by now, most people are using Python3.x. For this driver, only a few changes were necessary.

Copy link
Owner

@sim0nx sim0nx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Thanks for your PR.
"Most people", maybe but not all :-)
I would appreciate it if you could just add few small changes to your PR to make it PY2 and PY3 compatible.

In addition to what I posted in the review, also add this to tsl2561.py right below the header:

from future import absolute_import, division, print_function, unicode_literals

Thanks

@@ -7,7 +7,7 @@ Python library for TSL2561

Requirements
------------
- python 2.7.x
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just add py3 to the list, don't remove py2 completely

@@ -1,4 +1,4 @@
#!/usr/bin/env python
#!/usr/bin/python3
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If py3 is your default, the original header will still run it with py3, so don't force python3 here

@theFork
Copy link
Contributor Author

theFork commented Feb 6, 2017

Agreed. I made the changes you suggested.

@sim0nx
Copy link
Owner

sim0nx commented Feb 6, 2017

Thanks!

@sim0nx sim0nx merged commit ab2544c into sim0nx:master Feb 6, 2017
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