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

Pagination of results #70

Closed
schochastics opened this issue Nov 14, 2022 · 3 comments
Closed

Pagination of results #70

schochastics opened this issue Nov 14, 2022 · 3 comments
Assignees

Comments

@schochastics
Copy link
Member

A workaround is explained in the wiki but we need to built this in the package

@schochastics schochastics self-assigned this Nov 14, 2022
@schochastics
Copy link
Member Author

get_account_following <- function(id,max_id,since_id,limit = 40, token = NULL, parse = TRUE){
  path <- paste0("api/v1/accounts/",id,"/following")
  n <- limit   #CHANGE
  params <- list(limit = min(limit,40))    #CHANGE
  if (!missing(max_id)) {
    params$max_id <- max_id
  }
  if (!missing(since_id)) {
    params$since_id <- since_id
  }
  
  process_request1(token = token,path = path,
                  params = params,
                  parse = parse, FUN = v(parse_account),n = n,page_size = 40L) #CHANGE
}

process_request <- function(token = NULL,
                            path,
                            instance = NULL,
                            params,
                            anonymous = FALSE,
                            parse = TRUE,
                            FUN = identity,
                            n = 1,page_size=40L #CHANGE
){
 #CHANGE FROM
  pages <- ceiling(n/page_size)
  output <- vector("list")
  for(i in seq_len(pages)){
    tmp <- make_get_request(token = token,path = path,
                               instance = instance, params = params,
                               anonymous = anonymous)  
    output <- c(output,tmp)
    attr(output,"headers") <- attr(tmp,"headers")
    if(is.null(attr(tmp,"headers")[["max_id"]])){
      break
    }
    params[["max_id"]] <- attr(tmp,"headers")[["max_id"]]
  }
  #CHANGE TO
  if (isTRUE(parse)) {
    header <- attr(output,"headers")
    
    output <- FUN(output)
    attr(output,"headers") <- header
  }
  return(output)
}

@chainsawriot WIP of the pagination. Just to get some feedback (see #CHANGE). I try to not make the process visible at all for the user. New internal parameters would be n (the actual number of records to return) and page_size (the maximal records returned for an endpoint).
Few things:

  • rate limit checking is easy to add. Already have a wait function implemented.
  • So far only works with "max_id" (i.e. going backward). But I have a plan for since_id
  • Doesnt return the exact n, but multiples of the page_size. I think this is ok so we do not throw away fetched results (rtweet does that too)

What do you think?

@chainsawriot
Copy link
Collaborator

@schochastics As said in the httr's Best Practices, there are several ways to deal with this.

But from my view, it sounds okay. I would keep the current default. Mainly because I am lazy to create all the test cases again. Please try to make it works without breaking the current test cases.

@schochastics
Copy link
Member Author

Yes not breaking tests is my main goal besides not changing any defaults. I will not make in PR until I know that nothing breaks. Thanks for the feedback

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

2 participants