-
Notifications
You must be signed in to change notification settings - Fork 16.9k
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
core: Add ruff rules A (builtins shadowing) #29312
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
from langchain_core.messages import BaseMessage | ||
from langchain_core.output_parsers.transform import BaseTransformOutputParser | ||
|
||
T = TypeVar("T") | ||
|
||
|
||
def droplastn(iter: Iterator[T], n: int) -> Iterator[T]: | ||
def _droplastn(iterator: Iterator[T], n: int) -> Iterator[T]: |
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.
Is it OK to consider droplastn
private ?
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.
I generally suggest avoiding doing so unless there's a good reason -- we didn't have an official policy about the public interface, so it's hard to know what user code might be relying on, and unclear that there's a good reason to risk breaking some users code
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.
OK. I understand.
As a user though, I would really not use a function from output_parsers.list
in my own code...
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.
I rolled back. PTAL.
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.
Hi @eyurtsev could you take another look ?
ed51615
to
298117c
Compare
See https://docs.astral.sh/ruff/rules/#flake8-builtins-a
noqa
where backward compatibility was needed@override
when applicable