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

Raise limit to 100 in wrapper functions #22

Closed
peterdutey opened this issue Nov 19, 2021 · 3 comments
Closed

Raise limit to 100 in wrapper functions #22

peterdutey opened this issue Nov 19, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@peterdutey
Copy link
Contributor

peterdutey commented Nov 19, 2021

@AnikaC-git would you mind sense-checking the below before I implementing the proposed change in wrapper functions that use .split_into_chunks?

Source of the problem

concept_find() is designed to retrieve concepts by chunks of 99 conceptIds (see CHUNK_SIZE=100 in wrapper.R) from GET /{branch}/concepts. This is due to the URL character limit constraint of GET (:arrow_right: error 414 Request-URI Too Large)

But often the limit on the number of results sent back by snowstorm will be lower than this -- the default is 50, and a user cannot set limit above a hard threshold of 10,000.

Example

# Get 221 concept IDs
concepts <- concept_find(ecl = "<233604007", limit = 300)

# Retrieve those 
concepts_batch <- concept_find(
    conceptIds = concepts$conceptId,
    limit = 50
)

Warning messages:                                                                                                        
1: 
This server request returned just 50 of a total 99 results.
Please increase the server `limit` to fetch all results. 
2: 
This server request returned just 50 of a total 100 results.
Please increase the server `limit` to fetch all results. 

Currently, the function will display a warning with every chunk.
Say you are asking the function to retried 153 concepts, that means you have 2 chunks, so 2 warnings.
Neither elegant nor self-explanatory!
Users will not understand why this happens.

Note: The same thing will apply to:

  • concepts_included_in
  • concepts_descriptions
  • concepts_map
    For the latter 2, it's impossible to predict how many results will be returned for every chunk of size = 100.
    Hence forcing the limit to 10000. I don't anticipate this being insufficient, and even if it were, the user would see the warning (though they'd be unable to do much about it).
@peterdutey peterdutey added the bug Something isn't working label Nov 19, 2021
@peterdutey peterdutey added this to the 0.1.1 milestone Nov 19, 2021
@peterdutey peterdutey added this to To do in snomedizer development via automation Nov 19, 2021
@peterdutey peterdutey added enhancement New feature or request and removed bug Something isn't working labels Nov 21, 2021
@peterdutey peterdutey changed the title Raise default limit to 100 Raise limit to 100 in wrapper functions Nov 21, 2021
@peterdutey peterdutey modified the milestones: 0.2.0, 0.3.0 Dec 30, 2021
@peterdutey peterdutey moved this from To do to In progress in snomedizer development May 12, 2022
@AnikaC-git
Copy link
Collaborator

AnikaC-git commented May 18, 2022

I'm very sorry, but I am not sure I understand the problem fully with the information provided. So the API that the wrapper functions access has a query limit and this query limit cannot be changed? And therefore there is a splitter that basically splits results into smaller chunks?

@peterdutey
Copy link
Contributor Author

peterdutey commented May 18, 2022

No worries it's a little confusing.
Yes there is a limit on the number of results for every snowstorm API operation. By default it is set at 50. But a user can set the limit anywhere up to a maximum of 10000.

In addition, snomedizer wrapper functions use a chunking mechanism, for two reasons:

  • to get around the maximum 10000 hard limit on results
  • also to avoid issues when request parameters are too long, say when requesting too many concepts at once (some API operators accept arrays) that leads to exceeding the HTTP requrest URL character limit - hope that makes sense. I set this chunk size to 100 in snomedizer based on trial and error.

Hope this makes sense - happy to set a time and discuss!

@peterdutey
Copy link
Contributor Author

#27 merged today.

snomedizer development automation moved this from In progress to Done Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants