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

Refactor repetitive calls #48

Merged
merged 5 commits into from
Nov 9, 2022
Merged

Refactor repetitive calls #48

merged 5 commits into from
Nov 9, 2022

Conversation

schochastics
Copy link
Member

@schochastics schochastics commented Nov 8, 2022

This PR fixes the repetitive calls in all API calls with a function process_request()

It also fixes some path errors in
get_account_bookmarks() get_account_favourites() get_account_mutes() get_account_blocks()

Additionally, it exposes ratelimit and paging parameters as attributes

@chainsawriot happy to not merge if you think this interferes with your test setup too much. I did not run into problems so far
Your call!

R/utils.R Outdated
header <- attr(output,"headers")
if(length(output)<1){
output <- tibble::tibble()
} else if(length(output[[1]])==1){ #TODO:fragile?
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is indeed a bit fragile.

Copy link
Member Author

@schochastics schochastics Nov 9, 2022

Choose a reason for hiding this comment

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

Yeah trying to work out a better solution. Maybe check for depth of list?

# https://stackoverflow.com/a/13433689
depth <- function(this,thisdepth=0){
  if(!is.list(this)){
    return(thisdepth)
  }else{
    return(max(unlist(lapply(this,depth,thisdepth=thisdepth+1))))    
  }
} 

if depth =1 then we have only one object and if depth>1 we have a list of object?
Edit: Function needs tweaking. Doesnt work for our data
Edit: No this definitely doesn't work for our data. We need a different approach somehow

Copy link
Collaborator

@chainsawriot chainsawriot Nov 9, 2022

Choose a reason for hiding this comment

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

I think a better way to handle this is to use some crazy functional programming technique.

## vectorizing
v <- function(FUN) {
    v_FUN <- function(x) {
        dplyr::bind_rows(lapply(x, FUN))
    }
    return(v_FUN)
}

For the functions that need vectorization, pass, e.g. v(parse_status) as FUN to process_request

Copy link
Member Author

Choose a reason for hiding this comment

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

Holy! I try it out

Copy link
Member Author

Choose a reason for hiding this comment

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

but does it capture cases where we would usually expect several objects returned, but we only get one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it applies only to get_context. For that case, we just need to craft a specialize FUN for it.

So:

Function FUN
get_status parse_status
get_public_timeline v(parse_status)
get_context a specialize FUN

Copy link
Member Author

Choose a reason for hiding this comment

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

I test it

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/schochastics/rtoot/blob/89bed02988df595e3c767b661805450c39220b49/R/timelines_statuses.R#L97-L100

Something like

parse_context <- function(output) {
    temp_output <- list()
    temp_output$ancestors <- dplyr::bind_rows(lapply(output$ancestors, parse_status))
    temp_output$descendants <- dplyr::bind_rows(lapply(output$descendants, parse_status))
     temp_output
}

@chainsawriot
Copy link
Collaborator

Other minor points you can decide

  1. Should we still return headers if someone uses parse = FALSE?
  2. I think it is also time to move make_get_request to utils.R.

@schochastics
Copy link
Member Author

  1. for now I think we should. We can kick that out later
  2. yes thought that too and will do that

@schochastics
Copy link
Member Author

@chainsawriot done. Everything seems to work fine on my end

@chainsawriot
Copy link
Collaborator

Nice! I think it is fine now.

@schochastics
Copy link
Member Author

perfect I will also now update the wiki on pagination which should now work

@schochastics schochastics merged commit 7d359f0 into main Nov 9, 2022
@schochastics schochastics deleted the refactor_repetitive branch November 16, 2022 12:17
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.

2 participants