[DOC403/DOC502] False positive for abstract methods

[jsh9]

[DOC403/DOC502] False positive for abstract methods

[real-yfprojects]

• Either I care whether it's implemented as a generator or return statement, and I should use the Generator[...] type hint

• Or I don't care about it, and I should probably use the more generic Returns section in the docstring since there is no guarantee that the concrete implementation will actually yield something.

That's exactly what I was thinking.

[jsh9]

Thank you both for sharing useful feedback!

With the discussions above, I'm going to close this issue as "fixed". Please feel free to reopen it if any additional fixing is necessary.

[real-yfprojects]

I also do not understand with DOC201 is triggered in this case.

Pydoclint wants you to document that the method returns an Iterator which is different from yielding values which makes the method return a Generator. This is linked to #15.

[ghjklw]

I also do not understand with DOC201 is triggered in this case.

Pydoclint wants you to document that the method returns an Iterator which is different from yielding values which makes the method return a Generator. This is linked to #15.

Thanks! So I guess that's one more edge-case for DOC201 😉 The rule would be something like if a method is abstract and its output is either Iterable/Iterator/Generator and it contains a yield section, then DOC201 doesn't apply?

[real-yfprojects]

But why would you want to have a yield section but not a Generator return type?

[ghjklw]

For exactly the same reason as mentioned there: #15 (comment)
There is no reason that this combination of signature/docstring would work for a concrete method, but not for an abstract method?

[jsh9]

Hi @ghjklw:

Thank you for raising this issue. This is indeed an edge case that I didn't think of.

Let me explain the "strange" behavior between your Example 1 and Example 2.

In Example 1, pydoclint raises DOC201 and DOC403 because: it sees a Yields section in the docstring, so it's expecting a yield statement in the method body, not in the method signature.

This is because: when we see -> Iterator[something] in the signature, we can't be sure that the method is yielding stuff or returning stuff -- see my example here (#15 (comment)). Therefore, the yield statement in the method body is our only clue.

The situation is different for Example 2, where there is -> str in the signature. Since the return type annotation is not Iterator[...], we are certain that this method should return stuff rather than yield stuff.

That's why I wrote (hasReturnStmt or hasReturnAnno) in the code:

pydoclint/pydoclint/visitor.py

Lines 449 to 450 in 31e6a0b

	if docstringHasReturnSection and not (hasReturnStmt or hasReturnAnno):
	violations.append(v202)

So I actually don't know what the best solution should be: are we OK that both -> Iterator[...] and -> Generator[...] in abstract methods can correspond to a Yields section in the docstring? (If we are OK with this, this example will fall through the cracks.)

[jsh9]

Hi @ghjklw :

I merged PR #35. Please feel free to take a look at the code change, if you are curious.

Please note that your 1st example will still trigger violations, because if the return type annotation is Iterator[...], the linter still expects a return section, rather than a yield section.

If you have better suggestions on how the linter should handle Iterator/Generator and yields, please feel free to share them!

[ghjklw]

Thanks! I think that's a good compromise. Thinking again about this example I think @real-yfprojects has a point regarding the use of Iterator[...] in an abstract class or protocol:

• Either I care whether it's implemented as a generator or return statement, and I should use the Generator[...] type hint
• Or I don't care about it, and I should probably use the more generic Returns section in the docstring since there is no guarantee that the concrete implementation will actually yield something.

By the way, thanks for all the work you're doing ! It's fantastic to have such a tool 😃 I only hope I had the skills to build a VS Code extension for it 😄 !