Skip to content

Conversation

mskarlin
Copy link
Collaborator

@mskarlin mskarlin commented Dec 23, 2024

This upstreams FutureHouse's clinical trials search tool to make it open source. The tool is not turned on by default, but I'm going to make more docs on how to use the tool as well.

I also found many typing regressions -- I've added those back in where necessary.

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement New feature or request labels Dec 23, 2024
},
) as response:
if response.status == MALFORMATTED_QUERY_STATUS:
# the 400s from clinicaltrials.gov are not JSON
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind moving this comment to live with MALFORMATTED_QUERY_STATUS, or renaming MALFORMATTED_QUERY_STATUS to be CLINICAL_TRIALS_DIDNT_GIVE_JSON = 400

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But this comment is about the decoding, not the status (i.e. the code being 400 means it's malformed but the clinicaltrials.gov response isn't JSON in that case), so it really applies to running .json() vs. .text() on the response object.

Copy link
Collaborator

@jamesbraza jamesbraza left a comment

Choose a reason for hiding this comment

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

LGTM, great work here!!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 2, 2025
@mskarlin mskarlin merged commit 919bf0c into main Jan 2, 2025
5 checks passed
@mskarlin mskarlin deleted the upstream-clinical-trials branch January 2, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants