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

Generalize onlyInterest #439

Open
MaelAstruc opened this issue Nov 28, 2022 · 6 comments
Open

Generalize onlyInterest #439

MaelAstruc opened this issue Nov 28, 2022 · 6 comments

Comments

@MaelAstruc
Copy link

Hello,

Like others I have encountered issues with a Status code 429 #431 due to too many requests. To decrease the number of requests, I would like to only request what I need: the times variations or the regional variations.

For this, I could add a parameter outcomes in gtrendsR() similar to onlyInterest, which can be equal to:

  • "all" by default for all the information
  • "time" for the interest over time
  • "region" for the interest across regions
  • "topics" for the related topics
  • "queries" for the related queries

We could ask for multiple outcomes like "time" and "region" if we want only these two. The rest of the list would be filled with NULLs.

If this seems fine to you, I can create a fork and submit a PR with these modifications.

@eddelbuettel
Copy link
Collaborator

A request is still a request, even if you ask for fewer (sub-)results. The only way to have few requests is too ... make fewer requests.

It's a free api. It runs on their rules.

@MaelAstruc
Copy link
Author

If I understand the function and the API, there is at least one request for each outcome, isn't it ? More precisely, there is one request everytime the function curl::curl_fetch_memory() is called. So:

  • get_api_cookies : 1 request which is unavoidable
  • get_widget : 1 request which is unavoidable
  • interest_over_time : 1 request
  • create_geo_payload : 1 request per call, with one call per resolution
  • create_related_queries_payload : 1 request per call, with multiple calls
  • create_related_topics_payload: 1 request per call, with multiple calls

If this is the case, then following my previous example, if we are not interested in the related topics and queries, we could avoid doing requests for these two outcomes.

Is this correct or did I misunderstand how the functions work ?

@PMassicotte
Copy link
Owner

Just want to say that I am a bit overwhelmed with work this week. I will have a look soon.

@MaelAstruc
Copy link
Author

Thank you for looking at it.

I created a fork with my proposition if you want to take a look: https://github.com/MaelAstruc/gtrendsR

@eddelbuettel
Copy link
Collaborator

Is this correct or did I misunderstand how the functions work ?

Looks like you are/were quite correct and I was wrong, my bad! I have a vague recollection of us at some point only firing one query. So yes there may be scope for improvement (though it may also be tricky to do that in a clean 'backward-compatible" way).

@MaelAstruc
Copy link
Author

In terms of backward compatibility

  • My changes only divide the onlyInterest condition in multiple conditions, the core of the function remains the same.
  • I keep the onlyInterest argument but add a warning saying that it is deprecated, so previous code can still use it for now but there will be a warning.
  • The output remains the same, except for the case when onlyInterest was true. For those who did not use it, this has no consequences.
  • For the ones who use it, the results is also a list with "interest_over_time" as the first element, but there are also 6 elements equal to NULL afterwards. Hence, this is a breaking change in some strange cases, for example if the user checks that the length of the result is equal to 1 after using onlyInterest = TRUE.

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

3 participants