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

RUF045 on dataclass method assignment #16360

Open
beskep opened this issue Feb 25, 2025 · 5 comments
Open

RUF045 on dataclass method assignment #16360

beskep opened this issue Feb 25, 2025 · 5 comments
Labels
needs-decision Awaiting a decision from a maintainer preview Related to preview mode features rule Implementing or modifying a lint rule

Comments

@beskep
Copy link

beskep commented Feb 25, 2025

Description

ruff version: 0.9.7
command: ruff check test.py --select RUF --preview

import functools
from collections.abc import Callable
from dataclasses import dataclass


@dataclass
class Vector:
    x: float
    y: float

    def __call__(self) -> None:
        print(self)

    # RUF045: Assignment without annotation found in dataclass body
    call = __call__

    call2: Callable[..., None] = __call__  # this makes new field

    def add(self, x: float, y: float):
        return Vector(x=self.x + x, y=self.y + y)

    # RUF045: Assignment without annotation found in dataclass body
    add_x = functools.partialmethod(add, y=0)

I am not sure if it's intended, but ruff check RUF045 for assigning a method of dataclass.

Is there a more appropriate way for typing new method, perhaps?

@InSyncWithFoo
Copy link
Contributor

@dylwil3 This was a potential problem I noticed. What do you think, as the approver of that PR?

@dhruvmanila dhruvmanila added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer preview Related to preview mode features labels Feb 25, 2025
@dylwil3
Copy link
Collaborator

dylwil3 commented Feb 25, 2025

Is there a more appropriate way for typing new method, perhaps?

The suggestion in the documentation and "help" text of the rule is still correct: annotating the assignment with ClassVar[Callable] will give the behavior you're looking for.

I am not sure if it's intended, but ruff check RUF045 for assigning a method of dataclass.

@dylwil3 This was a potential problem I noticed. What do you think, as the approver of that PR?

Good questions!

I think the lint is correct to emit here: You can have dataclass fields that are typed as Callable and you can give them defaults; the help text of the diagnostic message is correct as given and the presence of the lint is consistent with the reasoning given in the documentation. I don't consider this a false positive and would argue in keeping this behavior even if we had stronger type inference around to avoid it.

In the discussion of the PR for this rule, I think there was some connection drawn to this issue around RUF012 and the comment:

This has the (maybe desirable, maybe not, but was surprising to me) effect of forcing typing for a RUF rule, in places where you would not necessarily use typing (if you did not want to).

But I really don't think those complaints apply to RUF045: dataclasses already enforce the use of typing, and in fact the main concern raised in that thread was about extending the scope of the rule to include classes other than dataclasses.

All that being said: It seems like the situation is unclear enough that the current issue was submitted. So I'm happy to be convinced I'm wrong here, and to hear other points of view!

@beskep
Copy link
Author

beskep commented Feb 26, 2025

add_x: ClassVar = functools.partialmethod(add, y=0) worked fine.
I had not thought of typing ClassVar for a function. How about adding this case to the example?

Or, perhaps, how about applying RUF045 only before def section?
It would increase false negatives though.

@InSyncWithFoo
Copy link
Contributor

The semantics of such a bare ClassVar annotation is currently unclear. Recently, a proposal was made to address this. It hasn't been accepted yet, however.

@beskep
Copy link
Author

beskep commented Feb 26, 2025

I appreciate RUF045 (I forget to type dataclass fields time to time), but ClassVar[Callable[..., None]] and ClassVar[partialmethod[Vector]] seems a bit verbose and cumbersome to me.
I'd rather mark noqa for those lines.

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 preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants