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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E1124:redundant-keyword-arg false positive on dataclass with kw_only=False that extends dataclass with kw_only=True #7290

Closed
DetachHead opened this issue Aug 11, 2022 · 5 comments · Fixed by pylint-dev/astroid#1764 or #7413
Assignees
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@DetachHead
Copy link
Contributor

DetachHead commented Aug 11, 2022

Bug description

import inspect
from dataclasses import dataclass


@dataclass(kw_only=True)
class Foo:
    a: int
    b: str


@dataclass(kw_only=False)
class Bar(Foo):
    c: int


print(inspect.signature(Bar.__init__))  # (self, c: int, *, a: int, b: str) -> None

a = Bar(1, a=2, b="")  # error: Argument 'a' passed by position and keyword in constructor call (E1124:redundant-keyword-arg)


print(a) # Bar(a=2, b='', c=1)

Configuration

No response

Command used

pylint asdf.py

Pylint output

************* Module asdf
asdf.py:18:4: E1124: Argument 'a' passed by position and keyword in constructor call (redundant-keyword-arg)

Expected behavior

no error

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.10.6 (tags/v3.10.6:9c7b4bd, Aug  1 2022, 21:53:49) [MSC v.1932 64 bit (AMD64)]

OS / Environment

No response

Additional dependencies

No response

@DetachHead DetachHead added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 11, 2022
@jacobtylerwalls
Copy link
Member

Thanks for the report. 👍🏻

I think this could be viewed as a sub-problem of #5767. Any fix for #5767 needs to handle this test case also. I'll cross-post there.

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Duplicate 🐫 Duplicate of an already existing issue and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 20, 2022
@DanielNoord
Copy link
Collaborator

Re-opening as I'd like to refocus #5767 to the KW_ONLY sentinel. Keeping this to track the issue of kw_only reordering the positional and keyword arguments needed for the dataclass constructor.

See also:


Happens with the kw_only parameter of dataclasses.field too:

"""Quick test to demonstrate pylint errors when using kw_only dataclass
fields."""
from dataclasses import dataclass, field

@dataclass
class Creature:
    """Defines a creature with one or more names (first names, middle names,
    last names, etc.)"""
    names: list[str] = field(kw_only=True)

@dataclass
class Human(Creature):
    """Defines a human who has a home address."""
    address: str

donna = Human('1858 Hidden Meadow Drive, East Grand Forks, ND 56721',
              names=['Donna', 'Jennings'])
print(donna)

Running pylint (2.12.2):

$ python -m pylint dc_kw_only.py 
************* Module dc_kw_only
dc_kw_only.py:16:8: E1124: Argument 'names' passed by position and keyword in constructor call (redundant-keyword-arg)
dc_kw_only.py:16:8: E1120: No value for argument 'address' in constructor call (no-value-for-parameter)

--------------------------------------------------------------------
Your code has been rated at -4.29/10

Running Python 3.10.2:

$ python dc_kw_only.py 
Human(names=['Donna', 'Jennings'], address='1858 Hidden Meadow Drive, East Grand Forks, ND 56721')

Originally posted by @Infernio in #5767 (comment)

@DanielNoord DanielNoord reopened this Aug 21, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Duplicate 🐫 Duplicate of an already existing issue labels Aug 22, 2022
@DanielNoord DanielNoord added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Sep 5, 2022
@DanielNoord DanielNoord self-assigned this Sep 5, 2022
@DanielNoord
Copy link
Collaborator

I posted a fix to astroid for this. Most issues with kw_only should be resolved with that PR.

@Infernio
Copy link

Infernio commented Oct 11, 2022

Finally got around to testing this. Unfortunately, I still get errors when using kw_only:

"""Docstring to quiet pylint."""
from dataclasses import dataclass, field

@dataclass
class ClassA:
    """Docstring to quiet pylint."""
    kw_field: str = field(kw_only=True)

@dataclass
class ClassB(ClassA):
    """Docstring to quiet pylint."""
    normal_field: int

print(ClassB(42, kw_field='Hello World'))

Runs fine on Python 3.10.7:

ClassB(kw_field='Hello World', normal_field=42)

But produces these errors when run through pylint (v2.15.4, with astroid v2.12.11):

************* Module demo
/tmp/demo.py:14:6: E1124: Argument 'kw_field' passed by position and keyword in constructor call (redundant-keyword-arg)
/tmp/demo.py:14:6: E1120: No value for argument 'normal_field' in constructor call (no-value-for-parameter)

Should I open a new issue for that?

@DanielNoord
Copy link
Collaborator

Can you open a new issue for this. Perhaps even in the astroid repo? That's where this ultimately should be fixed.

The issue is that we don't support kw_only in field (yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
5 participants