Add and improve type annotations

[oprypin]

No description provided.

[waylan]

In addition to the things I mentioned already, why do we need any annotations on private methods (starts with an underscore) or methods in subclasses which override the method with the same annotations in the parent class?

Thank you for this. It is a much more reasonable change. As some of my comments indicate, I find some things ridiculous, but am willing to overlook them as I didn't need to do the work to make the change myself. However, I do not expect to keep those annotations updated over time as they offer no value IMO.

[oprypin]

why do we need any annotations on private methods

Because I had no idea what the heck some arguments were meant to be when reading the code, and it was actually hard to figure out in many cases. Will save someone the work,

[waylan]

why do we need any annotations on private methods

Because I had no idea what the heck some arguments were meant to be when reading the code, and it was actually hard to figure out in many cases. Will save someone the work,

Here's the thing. Type annotations are documentation. Documentation is for users of the library. Users will never be accessing private methods (because they are private). So why do we need them....

To be clear, I actually dislike annotations, They are noise which makes the code harder to read. So when you include annotations were only the devs will ever use them, I don't like that. I would rather need to read the code to work out the type of a variable that have to separate the annotation from the code in my mind. But that may be just me. I'm not going to force that way on others.

In a similar vein, I do not care for the recent changes to the code comments. Those changes were made because the comments were being publisher on the site as API documentation. And so the language needed to be appropriate for that context. But personally, I prefer the terse comments which are directed at the devs of the code. I'm disappointed that we no longer have those. Therefore, on private methods, (which don't get included in the documentation) the original terse comments were left as-is.

[oprypin]

I don't follow your argument. Are you saying that private code doesn't need documentation? I, as a user, was reading private code on many occasions and could've used documentation.

[waylan]

I'm saying that a user will never call a private method, so no it does not need to be documented in the way that a public method would be. Of course, the method may need to have some documentation written for the developer of the code. However, that documentation can be more terse and make assumptions about the knowledge and understanding of the reader that would not be appropriate for user documentation. Sometimes, that documentation is nothing more than well chosen method and argument names. Other times, some comments are necessary as well.

As an example, take the case of a method which returns None. I think it is reasonable to assume that the developer of the code knows that a lack of a return statement will result in the method returning None. Therefore, the explicit return statement is not needed. Neither is an annotation or a comment. Such things are just noise which offer no value. However, if this is a method which may be called by a user, then those assumptions should not be made and it is necessary to add the noise via comments, annotations and maybe even a return statement.

I prefer that terse documentation in code comments. Even as a user of a library, when reading through unfamiliar code to understand how it works, I personally prefer that. And with nearly 20 years of reading Python code without type annotations, I see the annotations as noise.

Maybe I'm being a grump old guy here. But I'm just trying to explain my point of view. If I don't see any value in it, my first reaction is to reject it. So I ask, because maybe there is a point of view I'm missing.

If these changes are being suggested just so we can say the code has complete annotations and there is no other value in having them, then I'm inclined to so no thanks. But if there is real value, then that's fine. And I realize that just because I don't see the value that there isn't any. I just want to make sure there actually is and that someone isn't adding annotations because they think "everything must be annotated."

[oprypin]

I think it is reasonable to assume that the developer of the code knows that a lack of a return statement will result in the method returning None

Just take this one as an example

    def _build_row(self, row: str, parent: etree.Element, align: Sequence[str | None]) -> None:

It is instantly clear that, despite the name of the function, it doesn't return what it "built". (this is often a big one for me)
And it is clear that "align" is not a single item but something with N items. Each of which could be None as well.

Sure you could look and figure it out after a minute, but why?

You're saying that the developer who knows the code like the palm of their hand will have no trouble understanding the code. But what about everyone else?

[waylan]

I said that "Sometimes, that documentation is nothing more than well chosen method and argument names." However, you provided a counterexample where the names are not well chosen. Had they been, then yes, my point of view might hold up. I say "might" because I qualified my statement with "sometimes;" thereby acknowledging that that is not sufficient in every situation. But in the situations where is is sufficient, yes, anything extra is just noise.

Of course, the trick is working out the correct balance between what is sufficient and what is not. Each unique function/class/method is different and will have a different answer. Some developers will take the simple route and over-document everything. I prefer a more considered approach to avoid unnecessary noise. I am in no way saying that necessary information should be omitted, only unnecessary information.