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

Dataclass member documentation issues #411

Closed
JCGoran opened this issue Jul 6, 2022 · 6 comments
Closed

Dataclass member documentation issues #411

JCGoran opened this issue Jul 6, 2022 · 6 comments
Labels

Comments

@JCGoran
Copy link
Contributor

JCGoran commented Jul 6, 2022

Problem Description

When using the @dataclass decorator from the dataclasses module, pdoc seems to think that member (instance) variables with default values are actually class variables.
Furthermore, when using the typing.ClassVar hint, the class variable is not picked up by the parser (unless it has a default, but apparently specifying a default isn't a requirement for the code to run).

Steps to reproduce the behavior:

A minimal example is provided below:

from dataclasses import dataclass
from typing import ClassVar


@dataclass
class Example:
    value: float
    n: int = 1
    m: int = 2
    asdf: ClassVar


class OldExample:
    n: int = 2
    m: int = 2
    asdf: ClassVar

    def __init__(self):
        pass

Generating the documentation gives:
image
Note that neither n nor m should be shown in the docs for Example (unless they have docstrings attached, as per the docs), while asdf should always be shown in the docs as it is a class variable (as specified by its type hint), even if it has no value assigned (the code runs fine as long as one doesn't try to access asdf).

System Information

pdoc: 12.0.2
Python: 3.9.13
Platform: Linux-5.16.0-1-amd64-x86_64-with-glibc2.33
@JCGoran JCGoran added the bug label Jul 6, 2022
@mhils
Copy link
Member

mhils commented Jul 21, 2022

Hi there!

pdoc seems to think that member (instance) variables with default values are actually class variables.

What would you have expected to happen? From pdoc's perspective (and from Python's perspective) there is no real distinction between instance and class variables, so they all display the same. You can document you intent by using typing.ClassVar, which should show up in the type annotation.

the class variable is not picked up by the parser

I think this refers to an unrelated issue, namely the rules by which we pick up elements to document. Right now pdoc does not document variables if they only have an annotation but neither a docstring nor a default value (refs #280). I'm open to changes here. I'm not sure if it makes more sense to unconditionally display members in lieu of a docstring (nudging maintainers to add one), or to only display members if they have a docstring.

@JCGoran
Copy link
Contributor Author

JCGoran commented Jul 22, 2022

Hi there!

pdoc seems to think that member (instance) variables with default values are actually class variables.

What would you have expected to happen? From pdoc's perspective (and from Python's perspective) there is no real distinction between instance and class variables, so they all display the same. You can document you intent by using typing.ClassVar, which should show up in the type annotation.

I'm expecting that the instance variables of a dataclass with default values won't be displayed in the docs (unless I document them explicitly, as per the docs), because otherwise they are displayed twice, which is redundant and needlessly clutters the docs (I haven't found a way to remove them from the docs without ugly hacks, such as making the variables private, or rewriting the code so it doesn't use a dataclass at all).

the class variable is not picked up by the parser

I think this refers to an unrelated issue, namely the rules by which we pick up elements to document. Right now pdoc does not document variables if they only have an annotation but neither a docstring nor a default value (refs #280). I'm open to changes here. I'm not sure if it makes more sense to unconditionally display members in lieu of a docstring (nudging maintainers to add one), or to only display members if they have a docstring.

This is totally fine by me, if someone wishes to document a variable without a default value, they should explicitly specify a docstring, it's the (overzealous) documentation of variables that do have default values (but no docstrings) I have an issue with. The adage "Explicit is better than implicit" should apply in this case IMHO.

Side note: perhaps a new CLI option like --no-implicit-default-values could be introduced to maintain backwards compatibility? This way, users wanting this feature would need to just change one line in their doc-building scripts, and not touch their existing codebase.

@mhils
Copy link
Member

mhils commented Jul 22, 2022

because otherwise they are displayed twice

To clarify: You mean once as part of the constructor and once as a variable?

@JCGoran
Copy link
Contributor Author

JCGoran commented Jul 23, 2022

because otherwise they are displayed twice

To clarify: You mean once as part of the constructor and once as a variable?

Yes.

@JCGoran
Copy link
Contributor Author

JCGoran commented Aug 17, 2022

Any updates on this?

@mhils
Copy link
Member

mhils commented Sep 10, 2022

Fixed in #436!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants