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

removing the need for the six module while keeping Py2/3 compatibility #181

Closed
wants to merge 3 commits into from

Conversation

larkost
Copy link

@larkost larkost commented Dec 18, 2014

I noticed that there is relatively little use of the six module in the project, and these minor changes remove the need altogether.

from decimal import Decimal
from time import time as now

try:
from io import BytesIO
Copy link
Contributor

Choose a reason for hiding this comment

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

The io module works on Python 2.6+ and 3.0+ and has BytesIO on both. Why not just use that instead of incorrectly using StringIO?

Copy link
Author

Choose a reason for hiding this comment

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

From the presence of the six import I read implied pre-2.6 compatibility. If that is not needed then I can change this to be just from io import BytesIO. But as it is it has this effect on 2.6+.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see absolutely no reason to support 2.5 unless @johnsheehan has a different opinion

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to support pre 2.6.

Copy link
Contributor

Choose a reason for hiding this comment

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

2.6 and up is fine with me.

@johnsheehan
Copy link
Contributor

i'm not as well versed on six related issues as others so i'll leave this up to @sigmavirus24 to handle

@sigmavirus24
Copy link
Contributor

You should remove the dependency from setup.py too then.

@larkost
Copy link
Author

larkost commented Dec 26, 2014

Done

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

Successfully merging this pull request may close these issues.

None yet

5 participants