check-class-attributes=True does not work for attrs-class

[Waschenbacher]

It seems that check-class-attributes=True works for pure-python class but not for attrs-classes. Would it be possible to support the attrs-class with check-class-attributes=True as well? Thanks.

To reproduce:

"""Check pydoclint settings."""
from attrs import field, define


class Employee:
    """Class related to employee."""
    def __init__(self, first_name: str):
        self.first_name = first_name


@define
class EmployeeAttrs:
    """Class related to employee."""
    name: str = field()

pydoclint error message:

11: DOC601: Class `EmployeeAttrs`: Class docstring contains fewer class attributes than actual class attributes.  (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)
11: DOC603: Class `EmployeeAttrs`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [name: str]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.)

[jsh9]

Hi @Waschenbacher ,

I'm a bit confused, because I think the behavior of pydoclint is the correct behavior.

@define
class EmployeeAttrs:
    """Class related to employee."""
    name: str = field()

In this class def, name is a class attribute, so when we set --check-class-attributes=True, pydoclint expects us to document it in the class docstring, like this:

@define
class EmployeeAttrs:
    """
    Class related to employee.

    Attributes:
        name (str): Employee's name
    """
    name: str = field()

[AdrianSosic]

Hi @jsh9, for class-building frameworks like attrs (but also pydantic and the built-in dataclasses), this is unfortunately not correct, as they use the defined class only a blueprint that is translated in such a way that the class attributes from the original class become instance attributes in the final class. To create an actual class variable in attrs, you need to invoke the typing.ClassVar annotation.

Since attrs, pydantic, dataclasses and other similar tools are prevalent in the python world, I think it'd make sense to adjust pydoclint's logic here

[jsh9]

Hi @AdrianSosic and @Waschenbacher , sorry about the delayed response here.

In your opinion, what should the expected behavior of pydoclint be? Thanks!

[AdrianSosic]

I think the answer is rather simple here: the behavior should be identical to the one in the case of plain python classes, only the way class attributes are identified needs to be adjusted. That is, in all three cases, (attrs, dataclass, pydantic), an attribute only becomes a class variable when it is annotated with typing.ClassVar. So the check only needs to run on those. Does this help?

[jsh9]

I see. This makes sense.

Here is the logic that I have in mind:

I would add a new config option called --class-attributes-need-to-be-annotated-by-classvar (this is a mouthful, but if we can't find a succinct naming, we might as well use a whole sentence).

This option is by default False to maintain backward compatibility. If it's set to True, pydoclint would only check attributes that are annotated by typing.ClassVar. (Although, because pydoclint's name checking is static, or "dumb", I would only recognize the string ClassVar.)

What do you think of this logic?

---

Additionally, I have a question. In this example:

from typing import ClassVar

class MyClass:
    attr_1: ClassVar[int] = 0
    attr_2: int = 2

    def __init__(self) -> None:
        self.attr_3: ClassVar[str] = 'hello'

Is it correct to write self.attr_3: ClassVar[str] = 'hello' in the init function? And if so, shall pydoclint search the constructor (or other methods) to look for ClassVars? (I think the answers are "no and no", but I'd like to get your confirmation, since I haven't been very familiar with the ClassVar annotation.)

[AdrianSosic]

Hi @jsh9, writing from my phone hence I've very limited capabilities here. But I think perhaps I haven't made myself perfectly clear because it appears to me you are mixing up a few things. The ClassVar behavior is nothing intrinsically pythonic but is only interpreted by class-building frameworks like the ones I mentioned. Without using these them, the annotation has no effect (like annotations in general). That said, I think it also doesn't make much sense to talk about your example since it doesn't use any of the frameworks and hence all you attributes defined in the class header are class attributes, since you've created a plain python class in the regular way. Also the question about the constructor becomes somewhat obsolete since the frameworks replace the use of constructors (unless explicitly declared in special cases).

Therefore: what we are talking about here is not a question on backwards compatibility -- you can use regular python classes and the auto-created ones at the same time and there will be no ambiguities. So the question rather becomes: how can you reliably check if a dataclass-like mechanisms is in place? Here on my phone it's hard to check but afaik there is a PEP that now officially supports dataclass related interfaces. Potentially one could follow that approach. Can try to find it once I sent this message or next week when I'm back at work.

[AdrianSosic]

Got it:
https://peps.python.org/pep-0681/

Perhaps this offers a solution

[jsh9]

Hi @AdrianSosic , maybe you could show me examples for attrs, pydantic, dataclasses? Specifically, the code snippet and the expected violations (or lack of violations).

This could help me better understand this. Thanks!

[AdrianSosic]

Sure, can do next week when I'm back. In case I forget, feel free to ping me 👍

[jsh9]

Hi @AdrianSosic , did you happen to have some time to look at this? Thanks!

[AdrianSosic]

Hi @jsh9, thanks for the reminder, I knew I would forget 😅

Here the code snippets for the three frameworks, all implementing the identical logic.

from dataclasses import dataclass
from typing import ClassVar

from attrs import define, field
from pydantic import BaseModel, Field


@define
class AttrsClass:
    c: ClassVar[bool] = True  # class attribute
    x: int  # instance attribute
    y: float = 1.0  # instance attribute
    z: str = field(default="abc")  # instance attribute


@dataclass
class DataClass:
    c: ClassVar[bool] = True  # class attribute
    x: int  # instance attribute
    y: float = 1.0  # instance attribute
    z: str = field(default="abc")  # instance attribute


class PydanticClass(BaseModel):
    c: ClassVar[bool] = True  # class attribute
    x: int  # instance attribute
    y: float = 1.0  # instance attribute
    z: str = Field(default="abc")  # instance attribute

Note that only c is a class variable, all others become instance attributes after having been processed by the respective framework. Regarding documentation: for attrs and dataclass classes, it is common to document the attributes by attaching docstrings directly to the attributes, like:

@define
class WithDocstring:
    x: int
    """This is an int."""

I guess the same is true for pydantic, but I don't have much experience what are the best practices here.

Does that help?

[jsh9]

Hi @Waschenbacher and @AdrianSosic ,

I made a code change to add a new config option --only-attrs-with-ClassVar-are-treated-as-class-attrs. It's by default False. When it's set to True, pydoclint will treat attributes whose type annotation is within ClassVar[] as class attributes. (Note that you'd need to do from typing import ClassVar for this to work. If you do import typing and then typing.ClassVar[...], it won't work because of this design choice.)

[Waschenbacher]

@jsh9 Thanks for the update!