-
Notifications
You must be signed in to change notification settings - Fork 136
Make GraphRAG search's query backwards compatible #97
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,17 @@ | |
|
||
### Added | ||
- Add optional custom_prompt arg to the Text2CypherRetriever class. | ||
|
||
|
||
### Changed | ||
- `GraphRAG.search` method first parameter has been renamed `query_text` (was `query`) for consistency with the retrievers interface. | ||
- Made `GraphRAG.search` method backwards compatible with the query parameter, raising warnings to encourage using query_text instead. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we need a line in changelog for that, maybe rephrasing the previous one would be nicer. Something like:
|
||
|
||
## 0.3.1 | ||
|
||
### Fixed | ||
- Corrected initialization to allow specifying the embedding model name. | ||
- Removed sentence_transformers from embeddings/__init__.py to avoid ImportError when the package is not installed. | ||
|
||
### Changed | ||
- `GraphRAG.search` method first parameter has been renamed `query_text` (was `query`) for consistency with the retrievers interface. | ||
|
||
## 0.3.0 | ||
|
||
### Added | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
from __future__ import annotations | ||
|
||
import logging | ||
import warnings | ||
from typing import Any, Optional | ||
|
||
from pydantic import ValidationError | ||
|
@@ -53,10 +54,11 @@ def __init__( | |
|
||
def search( | ||
self, | ||
query_text: str, | ||
query_text: str = "", | ||
examples: str = "", | ||
retriever_config: Optional[dict[str, Any]] = None, | ||
return_context: bool = False, | ||
query: Optional[str] = None, | ||
) -> RagResultModel: | ||
"""This method performs a full RAG search: | ||
1. Retrieval: context retrieval | ||
|
@@ -69,12 +71,28 @@ def search( | |
retriever_config (Optional[dict]): Parameters passed to the retriever | ||
search method; e.g.: top_k | ||
return_context (bool): Whether to return the retriever result (default: False) | ||
query (Optional[str]): The user question. Will be deprecated in favor of query_text. | ||
|
||
Returns: | ||
RagResultModel: The LLM-generated answer | ||
|
||
""" | ||
try: | ||
if query is not None: | ||
if query_text: | ||
warnings.warn( | ||
"Both 'query' and 'query_text' are provided, 'query_text' will be used.", | ||
DeprecationWarning, | ||
stacklevel=2, | ||
) | ||
elif isinstance(query, str): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to test for the type of |
||
warnings.warn( | ||
"'query' is deprecated and will be removed in a future version, please use 'query_text' instead.", | ||
DeprecationWarning, | ||
stacklevel=2, | ||
) | ||
query_text = query | ||
|
||
validated_data = RagSearchModel( | ||
query_text=query_text, | ||
examples=examples, | ||
|
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.
Shall we add a "Deprecation notice" section to make it even more visible?