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

Add python six module for python compatibility #20

Merged
merged 4 commits into from
May 16, 2018
Merged

Conversation

lynneatan
Copy link
Contributor

@lynneatan lynneatan commented May 14, 2018

Addressing issue #19 Error in extract_cursor() method (#19)
Problem:

  • Users using python 2.7 can't retrieve all pledges to a campaign because the api returns a dict with unicode instead of string which causes an error to be thrown when iterating through a creator's pages ( see api.py line 73 for reference)

Solution:

  • Import the python six module that allows the conditional to handle both unicode and string (six.text_type)
  • Also removed the urllib_parse file and replaced it with six imports instead

@lynneatan lynneatan self-assigned this May 14, 2018
@lynneatan lynneatan requested a review from jrsun May 14, 2018 17:51
@@ -1,11 +1,12 @@
import requests
import six

from patreon.jsonapi.parser import JSONAPIParser
from patreon.jsonapi.url_util import build_url
from patreon.schemas import campaign
from patreon.utils import user_agent_string
from patreon.version_compatibility.utc_timezone import utc_timezone
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you get this from six.moves as well? Maybe version_compatibility can go away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I unfortunately didn't find any six documentation for timezone compatibility

@emosesPatreon
Copy link
Contributor

I don't see a change to requirements.txt here. You'll want to update it with six. pip freeze -r requirements.txt should do it.

Copy link

@jrsun jrsun left a comment

Choose a reason for hiding this comment

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

tiny comment! otherwise looks great 💯

patreon/api.py Outdated
raise Exception(
'Provided cursor path did not result in a link', current_dict
)
if type(current_dict) != six.text_type:
Copy link

Choose a reason for hiding this comment

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

let's just replace type(current_dict) != str on line 73 with this check.

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

3 participants