Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Rule: Use Sequence instead of list to annotate arguments #11061

Open
jd-solanki opened this issue Apr 21, 2024 · 2 comments
Open

New Rule: Use Sequence instead of list to annotate arguments #11061

jd-solanki opened this issue Apr 21, 2024 · 2 comments
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule type-inference Requires more advanced type inference.

Comments

@jd-solanki
Copy link

jd-solanki commented Apr 21, 2024

Hi 馃憢馃徎

Thanks for the amazing & blazing fast ecosystem 馃

With the following code:

from pydantic import BaseModel

class Itembase(BaseModel):
    title: str

class ItemRead(Itembase): ...

def my_util(Itembase): ...

items = [ItemRead(title"my title")]

my_utils(items) # 馃毃 Type Error: `Type parameter "_T@list" is invariant, Consider switching from "list" to "Sequence" which is covariant`

I checked the typing module and found this quote:

Note that to annotate arguments, it is preferred to use an abstract collection type such as Sequence or Iterable rather than to use list or typing.List.

Hence, I'm raising an issue that'll enforce using Sequence instead of list

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer labels Apr 21, 2024
@twoertwein
Copy link

If ruff can analyze the minimally needed types/methods for arguments and, based on that, suggests widening the type (for example, from list to Sequence), that would be amazing!

A blanket recommendation without some guard rails to replace list with Sequence would probably produce too many false positives.

@mikeshardmind
Copy link

Changing list to Sequence is often incorrect. MutableSequence is less likely to be a problem, but even that can cause issue if you are relying on the sequence being an actual list somewhere else. This would be extremely disruptive in many code bases due to interactions with other code outside of someone's control (you can't call a function expecting a list when you only know it's a sequence, for example)

@zanieb zanieb added the type-inference Requires more advanced type inference. label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule type-inference Requires more advanced type inference.
Projects
None yet
Development

No branches or pull requests

5 participants