Skip to content

Conversation

@baskaryan
Copy link
Collaborator

Doesn't actually limit the Retriever interface but hopefully in practice it does

@vercel
Copy link

vercel bot commented Jul 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jul 2, 2023 2:11pm

@dosubot dosubot bot added the auto:refactor label Jul 1, 2023

@abstractmethod
def get_relevant_documents(
self, query: str, *, callbacks: Callbacks = None, **kwargs: Any
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to think a bit more about backwards compatibility here

*,
run_manager: Optional[AsyncCallbackManagerForRetrieverRun],
**kwargs: Any,
self, query: str, *, run_manager: Optional[AsyncCallbackManagerForRetrieverRun]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticing there's decent amount of classes where run_manager is typed as Optional, some where it has a default val of None. on base it's not optional and has no default. guessing discrepancy is unintentional @hinthornw @nfcampos?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will save updating that for a sep pr

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be required. I had originally incorrectly made optional

@baskaryan baskaryan merged commit 7acd524 into master Jul 2, 2023
@baskaryan baskaryan deleted the bagatur/rm_retriever_kwargs branch July 2, 2023 14:22
bdonkey added a commit to bdonkey/langchain that referenced this pull request Jul 3, 2023
* master: (212 commits)
  Add SpacyEmbeddings class (langchain-ai#6967)
  docs: commented out `editUrl` option (langchain-ai#6440)
  Remove duplicate mongodb integration doc (langchain-ai#7006)
  Update get_started.mdx (langchain-ai#7005)
  openapi chain nit (langchain-ai#7012)
  Fix sample in FAISS section (langchain-ai#7050)
  Fix typo in google_places_api.py (langchain-ai#7055)
  move base prompt to schema (langchain-ai#6995)
  added `Brave Search` document_loader (langchain-ai#6989)
  Add JSON Lines support to JSONLoader (langchain-ai#6913)
  Vectara upd2 (langchain-ai#6506)
  docstrings `document_loaders` 2 (langchain-ai#6890)
  docstrings `document_loaders` 1 (langchain-ai#6847)
  Added filter and delete all option to delete function in Pinecone integration, updated base VectorStore's delete function (langchain-ai#6876)
  bump 221 (langchain-ai#7047)
  Rm retriever kwargs (langchain-ai#7013)
  Polish reference docs (langchain-ai#7045)
  Support params on GoogleSearchApiWrapper (langchain-ai#6810) (langchain-ai#7014)
  Fix typo (langchain-ai#7023)
  Fix openai multi functions agent docs (langchain-ai#7028)
  ...
vowelparrot pushed a commit that referenced this pull request Jul 4, 2023
Doesn't actually limit the Retriever interface but hopefully in practice
it does
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.

3 participants