Incomplete attrs support

[AdrianSosic]

• griffe-fieldz version: 0.2.0
• Python version: 3.10.14
• Operating System: macOS

Description

Hi, thanks for creating and sharing this package 👍🏼 Having a proper attrs support would be awesome – in fact, this currently blocks me from using mkdocs/mkdocstrings for my package documentation. I have just tested the extension but, unfortunately, it is not "really" compatible with attrs. While the basic functionality works as promised, adding attribute docstrings breaks the output and using certain built-in attrs features raises errors. See summary below:

What I Did

Here the example code I tried:

from attrs import define, field


@define
class MyClass:
    my_attribute: int = field()
    """Docstring of my_attribute."""

    @my_attribute.default
    def _default_my_attribute(self) -> int:
        """Default factory for my_attribute."""
        return 0

    @my_attribute.validator
    def _validate_my_attribute(self, attribute, value) -> None:
        if value % 2 != 0:
            raise ValueError(f"{attribute.name} must be even.")

    def my_method(self):
        """Docstring of my_method."""

1. When I run mkdocs serve on the snippet, I get

   TypeError: MyClass._default_my_attribute() missing 1 required positional argument: 'self'

   So it seems having a default factory in the code is not supported – which unfortunately makes the extension unusable for most attrs code.
2. When removing the factory, the extension does it's job, but the generated output is a bit awkward. Note that I'm completely new to mkdocs/mkdocstrings/griffe, so I don't know which piece of the chain to blame here. This is what I get:
   
   First, it is strange that my_attribute is listed again after the parameters list. This seems to be caused by having the docstring below the attribute definition, which however is a common thing to do. Second, I'm unsure what to expect from the "description" field. Perhaps this is where the docstring is supposed to go?

Any feedback would be highly appreciated 🙃

[tlambert03]

hey @AdrianSosic and thanks for opening an issue.

I am admittedly less familiar with attrs than I am with pydantic and dataclasses, so it's very possible I overlooked or did it slightly wrong. perhaps you can help me out if I point you to the right spots.

First off, most of the logic is actually in fieldz (a package that tries to give a standard interface to all of these dataclass-like things). I suspect the error with the default here is caused by this code here:

https://github.com/pyapp-kit/fieldz/blob/f46421fa0d7fb8000edfe7eb5b709baafc89c4a8/src/fieldz/adapters/_attrs.py#L65-L82

which simply passes the default.factory callable into Field(default_factory=...). And since it's a decorated method, we need to bind it to the class it's coming from (using a lambda, partial, or types.MethodType). I hadn't seen that approach of defining a default factory before, my bad. Should be an easy fix...

but the generated output is a bit awkward

yeah, i agree, it's a bit klugy there. I have a feeling this is more on the mkdocstrings side of things, and we'll need to dig a bit deeper into how it decides what to show in order to determine the best fix. I'm curious as well, but will need to look

[tlambert03]

oh jeez... looking a bit more at the docs of attrs

Default factories can also be set using the factory argument to field(), and using a decorator. The method receives the partially initialized instance which enables you to base a default value on other attributes:

I can see that might be useful at runtime, but that's kind of a tough case for documentation. It's basically saying there is no default that one could put in the documentation, since the default essentially depends on the values of the fields that were defined before it, right? In other words, in your example, if you added another field:

@define
class MyClass:
    x: int
    my_attribute: int = field()
    """Docstring of my_attribute."""

    @my_attribute.default
    def _default_my_attribute(self) -> int:
        """Default factory for my_attribute."""
        return self.x*2

what would you want the documentation to say?

I think inspecting the body of the function to try to say "the default value for my_attribute is whatever the value of x is times 2" is probably beyond the scope of this package and of fieldz... would you be happy with a solution that just puts "default = " in the generated docs?

[AdrianSosic]

Hi @tlambert03. Thank you very much for the quick reply 🥇 Your support is much appreciated. The good news: I have a rather large attrs-based code base that I'm trying to migrate to mkdocs, so that could serve as a kind of testbed for attrs integration of griffe-fieldz. Therefore, if you are eager to get this running, I'm happy to conduct the corresponding tests on my end 👍🏼

Now back to the topic 🙃 Regarding the default factories that process other attributes: Note that this is not attrs-specific, i.e. the same mechanism is also available in pydantic. Here a code snippet taken from its docs:

class User(BaseModel):
    email: EmailStr
    username: str = Field(default_factory=lambda data: data['email'])

However, I don't see this as a big problem, tbh. And I agree that trying to infer the logic from the function body is completely impossible, since it can execute arbitrary code. Let's look at the (mutually exclusive) cases and see what the reasonable consequences could be. I'll take the attrs syntax as an example and explain what I consider the "most reasonable" action to take, not knowing the implied implementation details for griffe, though.

Plain default

• Code: x: int = 0 or x: int = field(default=0)
• Consequence: Literal value 0 should be displayed as default.

Factory provided as decorated method

• Code:

  @y.default
  def _y_default(self) -> int:
      """Twice the value of x."""
      return self.x * 2

• Consequence: Here, it makes most sense to me that the user provides the description as the method docstring, because that's exactly the purpose of the docstring.

Factory provided as lambda

• Code y: int = field(factory: random.randint(0, 1)) or y: int = field(default=Factory(lambda self: self.x * 2, takes_self=True))
• Consequences: All chances off. I think there is nothing we can do here other than indicating that the default is provided via a factory. So perhaps one could display some form of sentinel value representing the presence of an undocumented factory?

What do you think?

[tlambert03]

thanks for the feedback! this is helpful and I think I have a clear idea of how to move forward. Will try to find time in the next week or two.

[AdrianSosic]

This is great news. Let me know when there is something for me to test 👌