search

seermedical / seer-py

Python SDK for the Seer data platform
MIT License
27 stars 10 forks source link

Convert more queries to use graphql variables #110

Closed bjdoyle closed 3 years ago

bjdoyle commented 3 years ago

Converts more queries to use graphql variables rather than python string formatting. In particular, all queries which use get_paginated_response have been converted. Also adds test utility methods which make testing queries which use get_paginated_response easier, cutting out a lot of boilerplate.

Also includes a change to the query string for get_diary_insights as the existing one was found to not be consistent with the current schema.

As well as test methods, all changed queries have been tested live against the api, with the exception of get_study_ids_in_study_cohort and get_user_ids_in_user_cohort, for which I could not find suitable data to test against.

Note that these tests were to ensure that the queries run without error. It would be appreciated, if you use particular queries, if you could confirm that they still perform as expected, in particular that they are returning data in the structure that you expect.

Further improvements to testing are required, in particular that the interpolation of variables is not tested in the current tests as that happens when the query is run, which is currently mocked. I have not fixed that in this PR, I am still thinking of a way to do it. It would also be good to have integration tests which run the queries against actual data but again that is a job for the future.

bjdoyle commented 3 years ago

@seerwill I've accepted some of your suggestions, made some changes in response to others. I don't want to make changes in this PR that go beyond converting existing queries as-is to use query variables, as far as that has been possible, so some others I have not changed.

seerwill commented 3 years ago

I don't want to make changes in this PR that go beyond converting existing queries as-is to use query variables

Makes sense, I think this PR has just highlighted some of the odd parts of the code base, so I though it was worth noting.