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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix aggregations with text queries #752

Merged

Conversation

MrEarle
Copy link
Contributor

@MrEarle MrEarle commented Oct 19, 2023

This aims to fix the bug referred to in #606.

First time contributing here, so please let me know if I'm missing something or I'm not up to your standards 馃榿.

@roman-right
Copy link
Member

Hi @MrEarle ,
Thank you for the PR! Could you please take a look at tests? It looks like python3.7 is not supported

@MrEarle
Copy link
Contributor Author

MrEarle commented Oct 23, 2023

Oh, you're right! I'll fix it as soon as I have some time for it 馃憣馃徏

@MrEarle MrEarle marked this pull request as draft October 23, 2023 21:03
@MrEarle MrEarle force-pushed the fix/aggregations-with-text-queries branch from 92181ee to 5790630 Compare October 23, 2023 21:04
@MrEarle MrEarle marked this pull request as ready for review October 23, 2023 21:31
@MrEarle
Copy link
Contributor Author

MrEarle commented Oct 23, 2023

Fixed the tests. I was using list for typing instead of List.

@roman-right
Copy link
Member

Awesome! I'll play with it tomorrow and merge. Thank you!

@roman-right
Copy link
Member

Good job! And I like how you avoided Python comparison operations, as they are overridden.

@roman-right roman-right merged commit 82661f1 into BeanieODM:main Oct 25, 2023
21 checks passed
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.

None yet

2 participants