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

Added synonyms to solr #6922

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added synonyms to solr #6922

wants to merge 4 commits into from

Conversation

bicolino34
Copy link
Collaborator

@bicolino34 bicolino34 commented Aug 31, 2022

Closes #6635

Technical

Testing

To test locally:

  1. Spin up a gitpod instance by going to https://gitpod.io/#https://github.com/internetarchive/openlibrary/pull/6922 (gitpod currently not working -_-)
  2. Edit some books to include the keywords
  3. Try searching for them using the synonyms

To test on staging solr (staff only):

TODO

Screenshot

Stakeholders

@cdrini @popcar2

@bicolino34 bicolino34 marked this pull request as ready for review August 31, 2022 11:12
@cdrini cdrini self-assigned this Aug 31, 2022
Copy link
Contributor

@tfmorris tfmorris left a comment

Choose a reason for hiding this comment

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

Roman numerals and random Ukrainian abbreviations seems like an odd set of stuff to focus on as the highest priority.

Many of the Roman numerals, as well the English names, are incorrect. I've provided a few examples, but the whole thing needs review/revision.

conf/solr/conf/synonyms.txt Outdated Show resolved Hide resolved
conf/solr/conf/synonyms.txt Outdated Show resolved Hide resolved
conf/solr/conf/synonyms.txt Outdated Show resolved Hide resolved
conf/solr/conf/synonyms.txt Outdated Show resolved Hide resolved
conf/solr/conf/synonyms.txt Outdated Show resolved Hide resolved
@mekarpeles mekarpeles added the Priority: 2 Important, as time permits. [managed] label Sep 19, 2022
@cdrini
Copy link
Collaborator

cdrini commented Nov 28, 2022

Sorry for the delay on this folks, testing this is a little complicated so will require me having a chunk of time to dig into it !

Television, Televisions, TV, TVs
Volume, Vol., Vol
&, and
1, One, I
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should test this one; this might cause false positives. Adding Vol. 1, Vol. I might be a more robust approach? But needs some testing. If Conail has some time, he should be able to load this up to run it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Testing Priority: 2 Important, as time permits. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add solr support for synonyms for numbers/abbreviations
4 participants