-
Notifications
You must be signed in to change notification settings - Fork 1
Implement semantic search for posts and articles #55
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
Conversation
1f9c466 to
65c0c02
Compare
4dcdfd2 to
b2a51b4
Compare
| return self.title | ||
|
|
||
| def save(self, *args, **kwargs): | ||
| title_vec = T.encode(self.title) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all this codes deserves a function, then when you read it, I think it will more readable:
self.embedding = self.prepare_vector_information()
return super().save(*args, **kwargs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went a bit further here and created a new class to handle embeddings — I think we can do more with them (or delete it forever... whichever happens first).
blog/models.py
Outdated
| super().save(*args, **kwargs) | ||
|
|
||
| @classmethod | ||
| def search(cls, q, dmax=0.5): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why a class method? I think if search is going to use with the orm, like objest.search, should we use a custom manager: https://docs.djangoproject.com/en/5.2/topics/db/managers/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood
eduzen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i left some comments
|
Test comment |
This pull request introduces semantic search functionality for the blog, allowing users to search for posts and articles based on the meaning of their content, not just keywords.
Not related changes (I was too lazy to create another PR 🙈):
I'm not 100% convinced about these changes, but I think it would be a pity not to integrate them. For now, let's move forward with the implementation, and maybe later we can improve it.
What don’t I like?
save()methods on Article and Post feel redundant. The same goes forsearch().Last but not least: how should we handle migration code? Should migrations be generated in dev and then pushed to the repo, or generated in prod and then pushed? What’s the best approach and why?