Renames HybridQuery to AggregateHybridQuery #422
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the HybridQuery class to AggregateHybridQuery to better reflect its implementation using Redis aggregation queries, while maintaining backward compatibility through a deprecation wrapper.
- Renamed
HybridQuerytoAggregateHybridQuerywith proper deprecation warnings - Updated all tests and documentation to use the new class name
- Added comprehensive backward compatibility tests to ensure the old
HybridQuerystill works with deprecation warnings
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
redisvl/query/aggregate.py |
Renamed main class to AggregateHybridQuery and created HybridQuery as a backward compatibility wrapper with deprecation warnings |
redisvl/query/__init__.py |
Exported both AggregateHybridQuery and HybridQuery for backward compatibility |
tests/unit/test_aggregation_types.py |
Updated all test cases to use AggregateHybridQuery and added backward compatibility tests with deprecation warning verification |
tests/integration/test_aggregation.py |
Updated all integration tests to use AggregateHybridQuery and added backward compatibility test |
docs/user_guide/11_advanced_queries.ipynb |
Updated documentation to reference AggregateHybridQuery consistently throughout examples and explanations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
redisvl/query/aggregate.py
Outdated
| .. deprecated:: | ||
| HybridQuery is a backward compatibility wrapper around AggregateHybridQuery | ||
| and will eventually be replaced with a new hybrid query implementation. | ||
| to maintain current functionality please use AggregateHybridQuery directly.", |
There was a problem hiding this comment.
There's a misplaced comma and period at the end of the docstring. The line should end with a period, not directly.", (comma followed by quote and period).
| to maintain current functionality please use AggregateHybridQuery directly.", | |
| to maintain current functionality please use AggregateHybridQuery directly. |
| text = "with a for but and" # will all be removed as default stopwords | ||
| with pytest.raises(ValueError): | ||
| hybrid_query = HybridQuery( | ||
| hybrid_query = AggregateHybridQuery( |
There was a problem hiding this comment.
Variable hybrid_query is not used.
|
|
||
| with pytest.raises(TypeError): | ||
| hybrid_query = HybridQuery( | ||
| hybrid_query = AggregateHybridQuery( |
There was a problem hiding this comment.
Variable hybrid_query is not used.
|
|
||
| # Verify AggregateHybridQuery does not emit warnings | ||
| with assert_no_warnings(): | ||
| aggregate_query = AggregateHybridQuery( |
There was a problem hiding this comment.
Variable aggregate_query is not used.
| aggregate_query = AggregateHybridQuery( | |
| AggregateHybridQuery( |
|
|
||
| # Verify that creating another HybridQuery also warns | ||
| with pytest.warns(DeprecationWarning): | ||
| another_hybrid_query = HybridQuery( |
There was a problem hiding this comment.
Variable another_hybrid_query is not used.
| another_hybrid_query = HybridQuery( | |
| HybridQuery( |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .. deprecated:: | ||
| HybridQuery is a backward compatibility wrapper around AggregateHybridQuery | ||
| and will eventually be replaced with a new hybrid query implementation. | ||
| To maintain current functionality please use AggregateHybridQuery directly.", |
There was a problem hiding this comment.
Remove trailing comma and quotation mark. The docstring should end with a period, not \",.
| To maintain current functionality please use AggregateHybridQuery directly.", | |
| To maintain current functionality please use AggregateHybridQuery directly. |
No description provided.