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

Prototype #46

Merged
merged 12 commits into from
Jun 8, 2023
Merged

Prototype #46

merged 12 commits into from
Jun 8, 2023

Conversation

cbizon
Copy link
Contributor

@cbizon cbizon commented Mar 10, 2023

Consumes the synonym files created by the synonym_prototype branch of Babel.

The input structure is now quite different. Rather than a single document per (curie,label) pair, we now make one doc per curie. We remove the id and length parameters, and add in biolink types and a preferred name.

This has an extra benefit of making the lookup code simpler, and reducing 3 solr calls to 1.

Fixes #39, fixes #43.
Fixes #24 by returning a list sorted by ElasticSearch scores instead of a dictionary.
Fixes #32 by adding types and canonical information to NameRes.

@cbizon cbizon requested a review from gaurav March 10, 2023 21:41
@cbizon
Copy link
Contributor Author

cbizon commented Mar 11, 2023

It may not be worth reviewing yet @gaurav - I realized that I need to update the openapi examples and the documentation.

@gaurav
Copy link
Contributor

gaurav commented Mar 15, 2023

It may not be worth reviewing yet @gaurav - I realized that I need to update the openapi examples and the documentation.

Sounds good! Let me know once this is ready for review.

@cbizon
Copy link
Contributor Author

cbizon commented Mar 15, 2023

I think it is ready now - the docs will need to be updated for the load, but probably after the makefile is sorted, and I think the openapi doesn't have an example anyway.

@gaurav
Copy link
Contributor

gaurav commented Apr 26, 2023

  • Filtering by Biolink type doesn't seem to be working at the moment.
    • This is because I had accidentally indexed types as a LowerTextField; changing it to a string fixed this issue.

We now look in preferred_name as well as the names field, and the query
works much better without the '*' at the end of it.
@gaurav
Copy link
Contributor

gaurav commented Apr 27, 2023

@cbizon I've got this working now! I made some changes to the queries; could you please check them and make sure they still make sense? You can see all the changes made since your last commit by going here: 48aa659...prototype

I've left preferred_name as a string field for now, but I think we should change it into a LowerTextField so that we can boost results that include the query in the preferred_name as compared to the names list. What do you think?

Apart from that question, I'm good to merge!

Copy link
Contributor

@gaurav gaurav left a comment

Choose a reason for hiding this comment

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

Works for me! (You can try it out at http://name-resolution-sri-dev.apps.renci.org/docs within the RENCI VPN).

@gaurav
Copy link
Contributor

gaurav commented Apr 27, 2023

@gaurav gaurav mentioned this pull request May 2, 2023
@gaurav gaurav merged commit f853638 into master Jun 8, 2023
1 check passed
@gaurav gaurav deleted the prototype branch June 8, 2023 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants