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

Wheel dependencies on python3 #400

Closed
Moutix opened this issue Feb 27, 2018 · 6 comments
Closed

Wheel dependencies on python3 #400

Moutix opened this issue Feb 27, 2018 · 6 comments

Comments

@Moutix
Copy link
Contributor

Moutix commented Feb 27, 2018

Hello!

Since the build was compiled using python2.7, it seems that it breaks the conditional dependencies here https://github.com/stripe/stripe-python/blob/master/setup.py#L36

So for instance, when installing stripe on python3, we now have a dependency on simplejson

One easy fix is to use https://www.python.org/dev/peps/pep-0508/ to manage conditional dependencies.

thks!

@Moutix Moutix changed the title Wheel on python3 Wheel dependencies on python3 Feb 27, 2018
@ob-stripe
Copy link
Contributor

ob-stripe commented Feb 27, 2018

Hi @Ningirsu, good catch! I recently tweaked the build process to generate a single universal wheel, but failed to realize that this would include unnecessary dependencies in Python 3 environments.

Using PEP 508 markers is indubitably the right thing to do here. My only concern is that support for PEP 508 was introduced relatively recently in pip (8.1.2) -- I'm not entirely sure what happens on older pip versions.

Regarding simplejson specifically, I think we can simply drop the requirement entirely. AFAIK it was only a requirement on Python <= 2.5 and 3.0, both of which have not been supported in a long time. All supported Python versions should be able to use the standard library's json module.

@brandur-stripe wdyt?

@brandur-stripe
Copy link
Contributor

@Ningirsu Sorry about the trouble here, and thanks for adding the helpful remediation advice!

Regarding simplejson specifically, I think we can simply drop the requirement entirely. AFAIK it was only a requirement on Python <= 2.5 and 3.0, both of which have not been supported in a long time. All supported Python versions should be able to use the standard library's json module.

@ob-stripe +1 your assessment. 2.5's been deprecated since mid-2013 and it's time to move on. 2.6 and 2.7 both have a built-in json package, so it should be safe to drop simplejson completely.

I noticed that we have one more conditional check in setup.py on Python < 2.6 so that we can constrain requests and pull in ssl. Since again this is in place for 2.5, I think it's safe to drop this completely as well.

I'm on run this week so I can put a PR together.

@brandur-stripe
Copy link
Contributor

@Ningirsu And actually, your PR is so close to what we'd want anyway that if you want to tweak it a little bit, we can pull it in instead of one that I make.

Do you want to just drop the conditional dependencies in favor of the original install_requires=install_requires? Also, while you're at it maybe remove the Python 2.5 deprecation warning. For the same reason the universal wheel doesn't handle the original conditionals properly, I don't think this will trigger how it was intended to.

Thanks again!

@Moutix
Copy link
Contributor Author

Moutix commented Feb 27, 2018

@brandur-stripe Yep Sure! Just updated the PR

@ob-stripe
Copy link
Contributor

Fixed in 1.79.1. Thanks again for the report and the fix @Ningirsu!

@Moutix
Copy link
Contributor Author

Moutix commented Feb 27, 2018

Thanks to you for handling this so quickly!
I will now be able to revert my hotfix on the Sqreen backend :)

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

No branches or pull requests

3 participants