added option to cursor() to yeild "page" - ie: iterators over each page returned by twitter #282

Merged
drevicko merged 5 commits from cursor-pages into master 2014-03-05 07:49:40 -08:00
drevicko commented 2013-11-07 17:48:42 -08:00 (Migrated from github.com)

When using cursors of the twitter api, results come back in "pages", with each page returned from a separate api call. If you want to track how many api calls you're making (not a bad idea if twitter's rate limit status is being unreliable, as in my case), it's handy to know how many calls a cursor is making as you iterate through it.

This pull request adds an option to returnPages = False the cursor() function. If set to True, cursor() yields an iterator for each page returned by twitter.

When using cursors of the twitter api, results come back in "pages", with each page returned from a separate api call. If you want to track how many api calls you're making (not a bad idea if twitter's rate limit status is being unreliable, as in my case), it's handy to know how many calls a cursor is making as you iterate through it. This pull request adds an option to `returnPages = False` the `cursor()` function. If set to `True`, `cursor()` yields an iterator for each page returned by twitter.
jimrybarski commented 2013-11-07 17:51:10 -08:00 (Migrated from github.com)

Can't you just count how many times you loop over the generator?

Can't you just count how many times you loop over the generator?
drevicko commented 2013-11-07 18:06:40 -08:00 (Migrated from github.com)

with the current implementation of cursor(), you'll count the number of objects returned by twitter, not the number of api calls it took to get them - they come from twitter lots-at-a-time (eg: 5000 user id's when calling friends/ids.json

with the current implementation of `cursor()`, you'll count the number of objects returned by twitter, not the number of api calls it took to get them - they come from twitter lots-at-a-time (eg: 5000 user id's when calling `friends/ids.json`
michaelhelmick commented 2014-02-18 18:06:41 -08:00 (Migrated from github.com)

Hi! Sorry I haven't gotten around to looking at this (it's been too long :X) The easiest way to get this pull request up-to-date would probably be pull from master and then re-add your changes to cursor

Also, instead of returnPages; can you use return_pages (Per http://www.python.org/dev/peps/pep-0008/#function-names)

And if you could remove the space: right now you have returnPages = False and returnPages = returnPages where return_pages=False and return_pages=return_pages would go with PEP8 standards.

Sorry if this causes an inconvenience :( If you need any help, let me know! Hope to see this Pull Request updated so we can merge this functionality in!

Hi! Sorry I haven't gotten around to looking at this (it's been too long :X) The easiest way to get this pull request up-to-date would probably be pull from `master` and then re-add your changes to `cursor` Also, instead of `returnPages`; can you use `return_pages` (Per http://www.python.org/dev/peps/pep-0008/#function-names) And if you could remove the space: right now you have `returnPages = False` and `returnPages = returnPages` where `return_pages=False` and `return_pages=return_pages` would go with PEP8 standards. Sorry if this causes an inconvenience :( If you need any help, let me know! Hope to see this Pull Request updated so we can merge this functionality in!
coveralls commented 2014-02-20 15:32:59 -08:00 (Migrated from github.com)

Coverage Status

Coverage decreased (-0.19%) when pulling e1ee67192e on drevicko:cursor-pages into da3a0bb9fc on ryanmcgrath:master.

[![Coverage Status](https://coveralls.io/builds/536644/badge)](https://coveralls.io/builds/536644) Coverage decreased (-0.19%) when pulling **e1ee67192e37df7ff44337c9aaf8981ae0d93549 on drevicko:cursor-pages** into **da3a0bb9fcefcda58b09820bd0c224ca9ed64401 on ryanmcgrath:master**.
drevicko commented 2014-02-20 15:48:52 -08:00 (Migrated from github.com)

While we're here, did you play with the other pull request I put in #284?
I'll merge that with master in case you want to include it.

Yeah, I know how it is. I've got lots of 'real' work on at the moment, so procrasta-github is taking a bit of a back seat! ;)

Oh, and no inconvenience - it's only a few minutes to merge (:

While we're here, did you play with the other pull request I put in #284? I'll merge that with master in case you want to include it. Yeah, I know how it is. I've got lots of 'real' work on at the moment, so procrasta-github is taking a bit of a back seat! ;) Oh, and no inconvenience - it's only a few minutes to merge (:
michaelhelmick commented 2014-02-25 12:39:07 -08:00 (Migrated from github.com)

@drevicko Seems as though merging #284 broke this merge :(

@drevicko Seems as though merging #284 broke this merge :(
michaelhelmick commented 2014-03-03 08:55:23 -08:00 (Migrated from github.com)

@drevicko Just wanted to ping you again. Sorry about the inconvenience! 🍰

@drevicko Just wanted to ping you again. Sorry about the inconvenience! :cake:
drevicko commented 2014-03-04 18:43:22 -08:00 (Migrated from github.com)

that should do it.. I'm not really in a position to test it at the moment, but if travis is happy?

Something I noticed: in my production version of twython, I've changed some of the exception handling: I always raise an exception if I can't parse the response as json, even if twitter returns a 200, 201 or 202 status. I think it was causing me problems (maybe the api was broken or something? It's a while ago, and I can't remember clearly). It's on line 197 of api.py . I've not pushed it to github yet - might do that now if you want to have a look.

that should do it.. I'm not really in a position to test it at the moment, but if travis is happy? Something I noticed: in my production version of twython, I've changed some of the exception handling: I always raise an exception if I can't parse the response as json, even if twitter returns a 200, 201 or 202 status. I think it was causing me problems (maybe the api was broken or something? It's a while ago, and I can't remember clearly). It's on line 197 of api.py . I've not pushed it to github yet - might do that now if you want to have a look.
michaelhelmick commented 2014-03-05 07:49:35 -08:00 (Migrated from github.com)

I would maybe do another pull request for that fix. I'll merge this in and then test for myself over the next couple days and do a release next week! Thanks!

I would maybe do another pull request for that fix. I'll merge this in and then test for myself over the next couple days and do a release next week! Thanks!
drevicko commented 2014-03-07 03:56:43 -08:00 (Migrated from github.com)

glad to help out (:
and thanks for putting twython together :)

glad to help out (: and thanks for putting twython together :)
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: code/twython#282
No description provided.