quickwit-oss / tantivy-py

Python bindings for Tantivy
MIT License
275 stars 63 forks source link

Expose boost query #250

Closed alex-au-922 closed 5 months ago

alex-au-922 commented 5 months ago

Added BoostQuery class with tests.

Expected usage:

@staticmethod
def boost_query(query: Query, boost: float = 1.0) -> Query:
    pass

# If we specify a boost value
Query.boost_query(BooleanQuery(...), 2)

# Default boost value is 1
Query.boost_query(BooleanQuery(...))

Test suite (✅ for expected success case, ❌ for expected failure case)

alex-au-922 commented 5 months ago

The feature of default boost value could be more than essential for this library. Perhaps I can remove it if it is not desirable

cjrh commented 5 months ago

I agree - I think the boost value should always be supplied by the caller.

FYI we do already have a higher-level boost mechanism here: https://github.com/quickwit-oss/tantivy-py/blob/deb88ccdcdbbb1aad0bca3e691bc58bfaca23133/tests/tantivy_test.py#L98

It would be helpful to add to the docs a short example of when one would prefer to use the boost_query() method instead of the field_boosts parameter in parse_query(). Perhaps when building up a chain from a Query type that is not supported by the query parser language?

alex-au-922 commented 5 months ago

I agree - I think the boost value should always be supplied by the caller.

FYI we do already have a higher-level boost mechanism here:

https://github.com/quickwit-oss/tantivy-py/blob/deb88ccdcdbbb1aad0bca3e691bc58bfaca23133/tests/tantivy_test.py#L98

It would be helpful to add to the docs a short example of when one would prefer to use the boost_query() method instead of the field_boosts parameter in parse_query(). Perhaps when building up a chain from a Query type that is not supported by the query parser language?

I have resolved the conflicts and updated the boost query not to have default value.

There should be two more tests:

alex-au-922 commented 5 months ago

For the documentation, I am thinking should we add a new section under Building and Executing Queries, perhaps we should rename Building and Executing Queries section to something like Quickstart with QueryParser?

Currently we have a lot of implemented methods regarding the issue #20 and I think we should have a proper documentation for parameters accepted and usecases.

cjrh commented 5 months ago

Currently we have a lot of implemented methods regarding the issue https://github.com/quickwit-oss/tantivy-py/issues/20 and I think we should have a proper documentation for parameters accepted and usecases.

yep agree 💯

should we add a new section under Building and Executing Queries

Your suggestions lead me in the direction of:

What do you think?

We can merge this PR first anyway, and do such docs in a different PR.