psyinfra / onyo

text-based inventory system on top of git
ISC License
3 stars 5 forks source link

Normalize doc-string and function definition style #613

Closed TobiasKadelka closed 6 months ago

TobiasKadelka commented 6 months ago

The reason for this is obvious and it resolving it has low priority. However, I notice it every time because it collides with my sense for aesthetics :P

Doc-strings:

Function-definitions:

If there is tooling that can unify this automatically that would be great. A lot of these points are very nitpicky, I collected them in this issue, because I noticed them while looking through #612

aqw commented 6 months ago

I view this issue as a place to discuss, rather than a tracking issue.

This variability in style is exactly what I am intending to address as begin my audit of all docstrings.

So far, I have only worked on argparse_helpers.py (#612).

We will follow numpy's formatting style.

sometimes we start in the first line where the """ are, other times in the line below.

This is called the short summary, and we will use it.

There are, of course, always exceptions. Those exceptions are for the cli/ commands, because they play by a different set of rules and those docstrings are for use in helptext and manpages.

the max-line length for docstrings is completely random everywhere, there seems to be no consence in onyo if they are 60, 80, or 100.

I have it wrap at 80 for the entire line, rather than the width of the text itself.

Trailing docs

I don't know what these are.

active/passive voice ("get" vs "gets") is not applied consistently

Active voice is indeed common best practice. That will be applied as well.

should the closing """ be in their own line, or the last line of the docstring?

According to numpy, it gets it's own line.

this is mainly important when multiple variables are given, but sometimes we still have them all in one line, sometimes we have one argument per line (which we decided on and which I prefer), but here we have some that start in the line of the function name itself, other times the first argument is in the line below.

I don't have a strong opinion on this.

TobiasKadelka commented 6 months ago

I view this issue as a place to discuss, rather than a tracking issue.

I think so too. And I agree with basically everything you say, especially:

We will follow numpy's formatting style.


Trailing docs

I don't know what these are.

This was a typo, I meant trailing dots -> . -- I adjust it above.

this is mainly important when multiple variables are given, but sometimes we still have them all in one line, sometimes we have one argument per line (which we decided on and which I prefer), but here we have some that start in the line of the function name itself, other times the first argument is in the line below.

I don't have a strong opinion on this.

I do. I liked Bens idea a few months ago that because of type hints, default values and everything, each argument should have its own line, and went with it. IMO it makes it way more readable (including when there are no type hints and default values, because then you see it instantly).

aqw commented 6 months ago

Trailing dots -> .

IMO, this is a relatively low value issue. But the general idea is that periods should only be used for full-sentences.

But then people say that such fragments should not be capitalized.

Which works great, until 1 string out of 20 needs another explaining sentence. Then it requires a period and capitalization. And then it looks different than all the rest.

So overall, I don't care that strongly. What I like is consistency. So I will enforce capitalization, and probably a trailing period.

I liked Bens idea a few months ago that because of type hints, default values and everything, each argument should have its own line,

If you like it and Ben proposed it, that's already 2/3 people. I'm happy to follow this.

TobiasKadelka commented 6 months ago

But the general idea is that periods should only be used for full-sentences.

According to whom? The example from the link you posted for short summaries above (https://numpydoc.readthedocs.io/en/latest/format.html#short-summary) has "The sum of two numbers.", which is no full sentence, but has a trailing dot and capitalization.

bpoldrack commented 6 months ago

Seems to me we are largely in agreement.

WRT to function declarations and code line lenght, I'd prefer to not be slavishly bound to anything. Generally I do put every argument in its own line, so it's instantly clear which one type annotations and possible defaults are associated with. However, if a function has two arguments with simple types and no defaults, I tend to prefer a more compact one liner. I don't think this needs to be overly strict. Line length: PEP8 states:

Some teams strongly prefer a longer line length. For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the line length limit up to 99 characters, provided that comments and docstrings are still wrapped at 72 characters.

So, I'd prefer that one over spending time trying to comply to a type writer legacy standard, which can worsen readability. However, if I feel it's nice and readable I'd still make it much shorter.

aqw commented 6 months ago

Cool. Sounds like we're in alignment.

So I will enforce capitalization, and probably a trailing period.

Seems that Tobias prefers this. I will apply it moving forward.