Cd 646 change show term count for unique terms to show counts#223
Cd 646 change show term count for unique terms to show counts#223dionboles-asym wants to merge 14 commits into
Conversation
… pagination bug (#221) * Updated unique terms to work with previous functionality, and fixed a pagination bug * updated test with new name and removed commented code * remove the test on remote * Updated descriptions for unique terms * updated show_counts to state that is nonfunctional
…t back to the expected values from the documentation. Additionally, updated the "paginator" and "get all" functions to accept "show_counts" and return the proper values for unique terms
| from tests.global_settings import host | ||
|
|
||
|
|
||
| # TODO debug paginator not returning show_counts with unique_terms |
There was a problem hiding this comment.
Is this TODO still relevant with the test on line 15?
There was a problem hiding this comment.
Nope i forgot to remove it
| verbose: bool = True, | ||
| limit: int = 100, | ||
| ) -> Paged_Result: | ||
| # TODO : Should the value be changed? The "show_counts" parameter was copied from Swagger-generated code. Is this description sufficient? |
There was a problem hiding this comment.
This TODO should be removed, right?
| Args: | ||
| col_name (str): _description_ | ||
| system (str, optional): _description_. Defaults to "". | ||
| col_name (str): This is the default way to search for a unique term from the CDA service API. |
There was a problem hiding this comment.
Thanks for filling out the docstring! It's been very helpful to guide our conversations.
| to_df (bool, optional): _description_. Defaults to False. | ||
| to_list (bool, optional): _description_. Defaults to False. | ||
| show_term_count (bool, optional): _description_. Defaults to False. | ||
| show_counts (bool, optional): _description_. Defaults to False. |
There was a problem hiding this comment.
docstring needs to be updated.
| async_req: bool, | ||
| include_total_count: bool, | ||
| show_term_count: Optional[bool], | ||
| show_counts: Optional[bool], |
There was a problem hiding this comment.
two questions:
- Is this needed on this factory or the other domain-type factories? (mutations, research_subject, subject, etc?). Do we use this value?
- Doc string at line 33 is incorrect and needs updating.
There was a problem hiding this comment.
In the factory, it is necessary to match the number of parameters in Q's 'Q._call_endpoint', which is currently being overrated by the factories. show in this error from pylint if show_counts is removed i can use a _ if we don't want to use show_counts Number of parameters was 8 in 'Q._call_endpoint' and is now 7 in overriding 'BooleanQuery._call_endpoint
There was a problem hiding this comment.
Right. Thanks for reminding me about the Q._call_endpoint.
The docstring below at line 33 needs updating.
…ient installation
…counts' of github.com:CancerDataAggregator/cda-python into CD-646-change-show_term_count-for-unique_terms-to-show_counts
4 participants
I removed the usage of "show_term_count" and replaced it with "show_counts" to restore the expected values according to the documentation. Moreover, I updated the "paginator" and "get all" functions to accept "show_counts" as a parameter and return the appropriate values for unique terms. Additionally, I introduced the "get_count" and "set_count" functions to the Q class. These functions allow setting the state of the Qconfig to be passed to the next Q call.