search

Health-Informatics-UoN / lettuce

MIT License
2 stars 0 forks source link

Feature/simple db multiple query #46

Closed kuraisle closed 2 months ago

kuraisle commented 2 months ago

Pull Request Contents

♻️ Refactor ✨ Feature

Description

I've modified OMOP_match so that OMOPMatcher.calculate_best_matches receives the list it was always meant to. This way, you can send a single API request, one connection is opened to the database, the queries are run, and the connection is closed.

I've changed the routes that use the database so the right format is used for OMOP queries. This doesn't work with the UI yet, but I'm keeping the PR small

Related Issues or other material

✅ Added/updated tests?

Added tests in the tests folder that check multiple queries return multiple results

Karthi-DStech commented 2 months ago

Hi James,

I tried to run the code and it closes the database connection as expected. But I can see, that it queries LLM directly and doesn't query the OMOP database with informal names before using the LLM.

Are you going to add this feature in this commit or should I want to add that?

kuraisle commented 2 months ago

Hi James,

I tried to run the code and it closes the database connection as expected. But I can see, that it queries LLM directly and doesn't query the OMOP database with informal names before using the LLM.

Are you going to add this feature in this commit or should I want to add that?

Hi Karthik,

To keep this to a single feature commit, let's add the other features separately. Did you try running the tests too? I thought it was pretty simple to set them up

Karthi-DStech commented 2 months ago

Ok James, let's implement the database login here and will create a new branch to implement the query first feature and an option to choose LLM in the GUI. I saw the test as well and it's working well. Good Job James👏🏻

kuraisle commented 2 months ago

Ok James, let's implement the database login here and will create a new branch to implement the query first feature and an option to choose LLM in the GUI. I saw the test as well and it's working well. Good Job James👏🏻

Is there any need for me to change anything?

Karthi-DStech commented 2 months ago

Is the GUI portion of the code working, James? The last time I tried running it, I encountered an error related to Streamlit.

kuraisle commented 2 months ago

Is the GUI portion of the code working, James? The last time I tried running it, I encountered an error related to Streamlit.

Nope, this breaks the GUI! I'll fix in another PR

Karthi-DStech commented 2 months ago

Alright James, I'll approve the PR.