-
Notifications
You must be signed in to change notification settings - Fork 16.2k
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
langchain[minor]: Add async methods to EncoderBackedStore #19597
langchain[minor]: Add async methods to EncoderBackedStore #19597
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
193873b
to
92189b0
Compare
def mset(self, key_value_pairs: Sequence[Tuple[K, V]]) -> None: | ||
"""Set the values for the given keys.""" |
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.
Looks good, but could you restore the comments? (Even if they are only marginally helpful. It's not obvious that folks will know what the abbrevation mset means)
Generally, we need to move in the direction of more in code documentation rather than less
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.
Since Python 3.5, docstrings are inherited : https://docs.python.org/3/library/inspect.html#inspect.getdoc
In IDEs (at least in PyCharm), the docstring is also inherited.
And it's also inherited in the API reference.
Removing docstrings reduces the amount of lines of code. And makes it easier to improve the doc of all subclasses by improving the parent.
Do you confirm that you prefer to duplicate the doc in subclasses nevertheless ?
updated doc-strings will merge |
OK. So I deduce that the policy for docstrings is to duplicate in the subclass ? |
…ai#19597) Co-authored-by: Eugene Yurtsev <[email protected]>
Co-authored-by: Eugene Yurtsev <[email protected]>
No description provided.