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

Google style: Allow documenting @propertys as a member #111

Closed
Tracked by #215
llucax opened this issue Jan 8, 2024 · 5 comments
Closed
Tracked by #215

Google style: Allow documenting @propertys as a member #111

llucax opened this issue Jan 8, 2024 · 5 comments

Comments

@llucax
Copy link
Contributor

llucax commented Jan 8, 2024

According to the Google style:

The docstring for a @Property data descriptor should use the same style as the docstring for an attribute or a function argument ("""The Bigtable path.""", rather than """Returns the Bigtable path.""").

For now this can be achieved by using the --skip-checking-short-docstrings but it is not exactly the same, as by using this option Args, Raises and other sections are also not required (not even for non-properties), which an lead to a lot of unintended missing documentation.

It would be good to have an option to make pydoclint explicitly support this style (omitting the Returns and the Args for setters).

As an extra nice feature, an option could be added to check this strictly, i.e. making it an error to have a Returns or Args in properties.

@jsh9
Copy link
Owner

jsh9 commented Jan 8, 2024

Just to confirm: in your opinion the ideal behavior for pydoclint should be "if this is a @property method, don't do any checking of input arguments, Returns section, and, Yields section, but still check the Raises section if the method raises any exception".

And an optional behavior is: "throw a violation if a @property method contains Returns, Args, and Yields sections" (Raises section can be permitted to exist).

Is my understanding correct?

@llucax
Copy link
Contributor Author

llucax commented Jan 9, 2024

Sort of. This is a bit beyond this issue in particular, but ideally I think that this, and any other "skip checking" option, I think if a section is included, it should be validated that it is consistent with the code. Not sure if --skip-checking-short-docstrings already works like this. So if it is like this, maybe it should be called --allow-missing-sections-in-short-docstrings or something like that.

Anyway, besides that detail, yes, what you say is correct.

I suggest something like:

  • --allow-missing-sections-in-property: If a section is missing, it won't complain (except for Raises). Existing sections are checked for correctness.
  • --forbid-sections-in-property: If a section is found in a property, it will emit an error (except for Raises).
  • Probably they should be mutually exclusive to avoid mistakes, otherwise --forbid-sections-in-property should probably win if both are present.

Considering interactions with #112 (comment):

  • --allow-missing-raises-for-short-docstrings is also used for both --allow-missing-sections-in-property and --forbid-sections-in-property, so if both are enabled and a property has only a short string, then Raises can be omitted too.

  • --allow-missing-args-for-short-docstrings is still used for properties too. If this option is enabled, none of the options above are enabled, and a property has only a short docstring but it is missing Args, no error should be emitted.

And with #110 (comment):

  • --allow-missing-xxx-if-starts-with-xxx is ignored for properties if any of --allow-missing-sections-in-property and --forbid-sections-in-property are used.

@llucax
Copy link
Contributor Author

llucax commented Jan 16, 2024

I guess this is partially fixed by #115, right?

@jsh9
Copy link
Owner

jsh9 commented Jan 29, 2024

Yes, #115 partially fixes this issue.

That said, I do not intend to pursue the other suggestions in this issue, because they could make the behaviors for the @property methods too complicated, hard to remember, and hence hard to maintain.

For @property methods, it is natural/intuitive that they don't need a return section in their docstring. So that's the only "exception" that I implemented.

@llucax
Copy link
Contributor Author

llucax commented Jan 31, 2024

I'm fine to consider this issue fixed with the changes in #115. This is enough to support google style for this particular case.

@jsh9 jsh9 closed this as completed Feb 11, 2024
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

No branches or pull requests

2 participants