Fixes #302 simplifies json decoding and handling of errors. Adds tests. #308

Merged

cash merged 2 commits from add_error_tests into master

2014-02-19 10:33:15 -08:00

cash commented

2014-01-25 14:03:08 -08:00

(Migrated from github.com)

This fixes the odd logic in the json decoding. I pushed the decoding burden onto the requests library.

I also cleaned up the logic for processing errors.

Unit tests added for the changes.

This fixes the odd logic in the json decoding. I pushed the decoding burden onto the requests library. I also cleaned up the logic for processing errors. Unit tests added for the changes.

coveralls commented

2014-01-25 14:17:38 -08:00

(Migrated from github.com)

Coverage increased (+1.21%) when pulling 1e627f9fb1 on cash:add_error_tests into eed37be5d1 on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/466461/badge)](https://coveralls.io/builds/466461) Coverage increased (+1.21%) when pulling **1e627f9fb1cd0bc06704739e4426f6d1218352ac on cash:add_error_tests** into **eed37be5d1c0520a00d611228c028a71069f7fe2 on ryanmcgrath:master**.

michaelhelmick commented

2014-01-29 14:03:06 -08:00

(Migrated from github.com)

Just so you know, this is on my radar, I've just had a hectic couple of weeks!

michaelhelmick commented

2014-02-18 17:59:55 -08:00

(Migrated from github.com)

@cash Sorry it's taken me so long to get to this (I've been super busy with work.. a lot going on ;P)

Anyways, this looks good to go, except -- as far as decoding goes. Does retrieving Japanese characters (or any foreign characters for that matter) work? The decode was for if someone was say.. searching a Twitter term of foreign, non-english, characters (@ryanmcgrath could probably give you an example, he's more familiar with the whole decoding/character stuff)

That's my only concern about this pull request. :) Again, good work, thanks!

@cash Sorry it's taken me so long to get to this (I've been super busy with work.. a lot going on ;P) Anyways, this looks good to go, except -- as far as decoding goes. Does retrieving Japanese characters (or any foreign characters for that matter) work? The decode was for if someone was say.. searching a Twitter term of foreign, non-english, characters (@ryanmcgrath could probably give you an example, he's more familiar with the whole decoding/character stuff) That's my only concern about this pull request. :) Again, good work, thanks!

cash commented

2014-02-18 19:56:06 -08:00

(Migrated from github.com)

I don't have a working unit test for utf-8 yet. Here is an example I ran on this branch:

tweet = twitter.get('statuses/show', {'id': '435789394440499200'})
print(tweet['text'])

Results:

『ライ麦畑でつかまえて』のライ麦って、どういう味？

Link to tweet: https://twitter.com/tunecan/status/435789394440499200

This works because of the requests library.
json() method: https://github.com/kennethreitz/requests/blob/master/requests/models.py#L739
text() method: https://github.com/kennethreitz/requests/blob/master/requests/models.py#L702

Requests uses chardet to detect the encoding. If you wanted to force 'utf-8', it just takes a single line after getting the response back:

response = func(url, **requests_args)
response.encoding = 'utf-8'

I don't think it is required because Twitter sets the charset in the header, but it also wouldn't hurt adding it.

I don't have a working unit test for utf-8 yet. Here is an example I ran on this branch: ``` tweet = twitter.get('statuses/show', {'id': '435789394440499200'}) print(tweet['text']) ``` Results: ``` 『ライ麦畑でつかまえて』のライ麦って、どういう味？ ``` Link to tweet: https://twitter.com/tunecan/status/435789394440499200 This works because of the requests library. json() method: https://github.com/kennethreitz/requests/blob/master/requests/models.py#L739 text() method: https://github.com/kennethreitz/requests/blob/master/requests/models.py#L702 Requests uses chardet to detect the encoding. If you wanted to force 'utf-8', it just takes a single line after getting the response back: ``` response = func(url, **requests_args) response.encoding = 'utf-8' ``` I don't think it is required because Twitter sets the charset in the header, but it also wouldn't hurt adding it.

michaelhelmick commented

2014-02-19 10:33:13 -08:00

(Migrated from github.com)

Ah! Solid, thanks!