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

bug: A dataclass field with init=False should not be a parameter #233

Closed
has2k1 opened this issue Feb 8, 2024 · 3 comments · Fixed by #240
Closed

bug: A dataclass field with init=False should not be a parameter #233

has2k1 opened this issue Feb 8, 2024 · 3 comments · Fixed by #240
Assignees

Comments

@has2k1
Copy link
Contributor

has2k1 commented Feb 8, 2024

Here are failing tests.

from griffe.tests import temporary_visited_module

code = """
from dataclasses import dataclass, field

@dataclass
class PointA:
    x: float
    y: float
    z: float = field(init=False)

@dataclass(init=False)
class PointB:
    x: float
    y: float

@dataclass(init=False)
class PointC:
    x: float
    y: float = field(init=True)  # init=True has no effect
"""

with temporary_visited_module(code) as module:
    paramsA = module["PointA"].parameters
    paramsB = module["PointB"].parameters
    paramsC = module["PointC"].parameters

    assert "z" not in paramsA
    assert "x" not in paramsB and "y" not in paramsB
    assert "x" not in paramsC and "y" not in paramsC

Then there are the perhaps more complicated cases to deal with statically where the field class is wrapped inside another function e.g.

def no_init(default: T) -> T:
    """
    Set default value of a dataclass field that will not be __init__ed
    """
    return field(init=False, default=default)

@dataclass
class Point:
    x: float
    y: float = no_init(0)  # y is not a parameter
@pawamoy
Copy link
Member

pawamoy commented Feb 10, 2024

Parameters for dataclasses are currently dynamically created when accessing the class' parameters property. Seeing that the logic can get quite complex, and based on the fact that this parameters property cannot be easily extended by users or extensions, I think we should change the logic and generate an __init__ method object when visiting class decorated with @dataclasses.dataclass. Extensions for third-party libs (Pydantic, attrs, whatever) could then do the same, without any special code in Griffe itself.

has2k1 added a commit to has2k1/qrenderer that referenced this issue Feb 10, 2024
@has2k1
Copy link
Contributor Author

has2k1 commented Feb 10, 2024

I haven't looked at the how the parameters are generated, but I have a temporary solution that inspects ExprCall parameters and I only do the check for dataclasses.

Obviously this does not handle the cases when field(init=False) is hidden inside another function.

@pawamoy
Copy link
Member

pawamoy commented Feb 10, 2024

Yes, that's what we'll use in Griffe too (thanks for the link). Just not in the parameters property as it's not extensible :)

Regarding indirections (hiding the field call in other functions), that won't be supported statically, and users will have to rely on dynamic analysis.

pawamoy added a commit that referenced this issue Mar 5, 2024
Instead of generating parameters on the fly by (wrongly) checking attributes of the class,
we always load a Griffe extension that re-creates `__init__` methods and their parameters.

Issue-33: #233
Issue-34: #234
Issue-38: #238
Issue-39: #239
PR-240: #240
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 a pull request may close this issue.

2 participants