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

Using ngrams for better search results #213

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

tibor
Copy link
Contributor

@tibor tibor commented Apr 15, 2023

Like in #212 descriped

@stefanw
Copy link
Owner

stefanw commented Apr 16, 2023

Das Konzept gefällt mir. Aber Text unter 5 Wörter liefern damit leeren Query zurück. Query darf insgesamt vermutlich auch nicht zu lang werden.

Der Code für queries liegt momentan auch nicht ordentlich strukturiert vor:

  • pro "Source" sollte es einen query generator geben, weil eine Quelle z.B. Sonderzeichen wie Guillemets nicht mag.
  • jede "Site" sollte einfach nur guten Text liefern, muss aber ggf. auch umformatiert werden können

Bevor das also reinkommt, würde ich da 1. noch Tests haben und

@stefanw stefanw closed this Apr 16, 2023
@stefanw stefanw reopened this Apr 16, 2023
Implemented fallback to always return some text
Implemented a text limit
@tibor
Copy link
Contributor Author

tibor commented Apr 17, 2023

Gutes Feedback, Danke!
Die beiden Punkte habe ich direkt einmal geändert. Bei der Textlänge bin ich bislang nicht auf Probleme gestoßen, macht aber trotzdem Sinn.

Die Queries finde ich einen guten Punkt. Ich würde zudem mehrere Selektoren pro Site zulassen, um sowohl Headline als auch Teaser suchen zu können.

Zu den Sonderzeichen: Hast du dafür Beispiele? Wenn die Guillemets schwierig sind, werden sonstige Sonderzeichen wahrscheinlich auch schwierig, oder?

Fehler bei der Wörterlänge behoben
@tibor
Copy link
Contributor Author

tibor commented Apr 18, 2023

Bei zeit.de ist mir noch ein Problem aufgefallen: Mit dem neuen Code wird dort eine leere Query an Genios übermittelt, mir ist aber nicht klar, warum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants