Function overload support and param/return-sections-undocumented change #595

cretz · 2022-06-02T12:05:43Z

Handle @overload decorator and situation where all params/returns are undocumented. Specifically:

Show signatures for @overload functions and add @overload decorator above them
Do not show main function signature if overloads are present
If all params are undocumented, do not show param table
If return is undocumented, do not show return table
Builds off of @overload decorator support and add types annotations to signatures #417 (only minor changes). Fixes Handle overloaded functions #406.

Example, for reST-format of:

@overload
def demo_overload(s: str) -> str:
    ...

@overload
def demo_overload(s: bytes) -> bytes:
    ...

def demo_overload(s: Union[str, bytes]) -> Union[str, bytes]:
    """
    Overload signatures appear without the main signature and with ``@overload`` decorator.

    :param s: Some string or bytes param.
    :return: Some string or bytes result.
    """

Result:

Note how (source) is below the signatures instead of to the right as it would be w/ just one signature. Seems to be an acceptable tradeoff for now.

For undocumented reST-format of:

def demo_undocumented(s: str) -> str:
    raise NotImplementedError

Result:

Notice the lack of parameter and return sections.

Here's an example of a really large overload from some source in my company's repo:

Yes it can appear kind of like a large mess, but there aren't really good alternatives and it's my company's fault for using so many args. I don't think we'd want to wrap param signatures nor do I think we'd want to do a diff and only have the overloads show what's unique to them.

Replaces and closes twisted#417, fixes twisted#406

tristanlatr · 2022-06-02T13:40:17Z

Hello @cretz,

I know the PR is marked as draft so you'll probably work on it again later, but I want to give a little heads-up about little stuff I saw that might be worth mentioning before going to far in the hacking:

Since the object FunctionOverload is not an actual documentable type, I wouldn't add it to the factory mechanism, since we could tweak the function overloads by applying a mixin to the Function class.

Also did you read this ticket about types in signatures ? #473

Thanks for you work on this already :)

Talk to you later,

cretz · 2022-06-02T13:50:00Z

@tristanlatr - Thanks! Yeah, I have it draft because I was (properly) afraid the CI checks here might not pass. I am fixing and then will open full review.

Since the object FunctionOverload is not an actual documentable type, I wouldn't add it to the factory mechanism, since we could tweak the function overloads by applying a mixin to the Function class.

Ok, I can remove the factory part for that

Also did you read this ticket about types in signatures ? #473

I am seeing it now. Seems like removing the entire parameters/return section if all are "Undocumented", correct? But otherwise leave as is? Usually I would say this should be a separate PR, but I have also been really wanting this (I have big shortcut methods that are documented as just "see whatever"). I am adding now.

codecov · 2022-06-02T15:03:35Z

Codecov Report

Base: 92.31% // Head: 92.39% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (1570950) compared to base (3c10774).
Patch coverage: 97.65% of modified lines in pull request are covered.

❗ Current head 1570950 differs from pull request most recent head 2a1bb77. Consider uploading reports for the commit 2a1bb77 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
+ Coverage   92.31%   92.39%   +0.08%     
==========================================
  Files          45       45              
  Lines        7959     8026      +67     
  Branches     1736     1750      +14     
==========================================
+ Hits         7347     7416      +69     
+ Misses        356      355       -1     
+ Partials      256      255       -1

Impacted Files	Coverage Δ
pydoctor/templatewriter/pages/__init__.py	`90.69% <90.47%> (-0.22%)`	⬇️
pydoctor/astutils.py	`92.81% <95.23%> (+0.32%)`	⬆️
pydoctor/astbuilder.py	`95.97% <100.00%> (+0.40%)`	⬆️
pydoctor/epydoc2stan.py	`93.83% <100.00%> (+0.15%)`	⬆️
pydoctor/model.py	`94.91% <100.00%> (+0.12%)`	⬆️
pydoctor/templatewriter/pages/functionchild.py	`97.95% <100.00%> (+3.95%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

tristanlatr

It's going to be a great improvement no to display undocumented EVERYWHERE.

I made a few comments that I believe are worth changing bits of the code. I know I wrote this code at first but it was about a year ago, and we now have a better colorizer for AST values that is worth using.

Also, I can write a couple of test cases for the edge cases if you want.

Thanks for your work,

Talk to you later,

pydoctor/astbuilder.py

pydoctor/epydoc2stan.py

pydoctor/model.py

pydoctor/templatewriter/pages/__init__.py

pydoctor/templatewriter/pages/functionchild.py

pydoctor/test/test_epydoc2stan.py

tristanlatr · 2022-06-02T16:31:41Z

pydoctor/test/test_epydoc2stan.py

@@ -239,8 +289,8 @@ def f(a, b):
    classic_fmt = docstring2html(classic_mod.contents['f'])
    assert annotation_fmt == classic_fmt

-@pytest.mark.xfail


I didn't know we had that!

tristanlatr · 2022-06-02T16:50:47Z

Also, could you include a few entries in the change log to summarize what's been changed in this PR? Thanks.

cretz · 2022-06-02T16:55:21Z

Will make updates, probably some time tomorrow

cretz · 2022-06-03T13:49:00Z

Updates made, can re-review (assuming CI checks pass, if not, will force-push whatever tiny changes needed)

tristanlatr

I think there is a little bug

pydoctor/templatewriter/pages/__init__.py

tristanlatr

Overhaul looks good, only minor suggestions.

pydoctor/templatewriter/pages/__init__.py

pydoctor/test/test_astbuilder.py

pydoctor/test/test_epydoc2stan.py

pydoctor/test/test_templatewriter.py

pydoctor/templatewriter/pages/__init__.py

pydoctor/themes/base/function-child.html

tristanlatr

Hello @cretz,

Thanks for your work on this, turns out it's more complicated than expected (and we're missing regression tests). I believe more reasoning is needed to honour @type: fields that don't have associated documentation.

Talk to you later,

pydoctor/test/test_epydoc2stan.py

tristanlatr · 2022-06-03T19:33:32Z

pydoctor/epydoc2stan.py

+        # Only include parameter or return sections if any are documented
+        if any(p.body for p in self.parameter_descs):


There is something that bothers me, here. (see other comment).

We need a way to determine of the type info is coming from AST, in which case we should display it only if it's documented or if the type is coming from docstring fields, in which case it should display anyhow (with undocumented notes). In terms of implementation, the resolve_types() might need an adjustment in order to track if the FieldDesc is create from resolve_types() (meaning we only have AST info), for instance by creating _BaseFieldDesc superclass of FieldDesc and AstFieldDesc and use that instead from resolve_types(). Then check for the existence of FieldDesc instances in self.parameter_descs.

Tell me if you think it could work,

Talk to you later,

This might now work since we want to track the origin of the type info, and this would only work to track undocumented params…

I believe adding a simple attribute FieldDesc.type_origin: Enum value - AST/DOCSTRING could make the trick.

We need a way to determine of the type info is coming from AST, in which case we should display it only if it's documented or if the type is coming from docstring fields, in which case it should display anyhow (with undocumented notes).

This seems a bit off. If someone has chosen to use @type (maybe to help their IDE or whatever) but have chosen not to use @param, they are choosing to not document, right?

It seems you are suggested that @type be treated as an empty @param doc? How would I, as a user that doesn't use @type, be able to get that empty documentation functionality the way those that do use @type get?

It seems you are suggested that @type be treated as an empty @param doc?

Otherwise we would just ignore it. Because free text can't be included in signatures, and should not.

How would I, as a user that doesn't use @type, be able to get that empty documentation functionality the way those that do use @type get?

You are raising valid points here.

The simplest thing we could do when a @type field is alone with no documentation (either coming from @param or @var, @ivar or @cvar) it to report a warning saying that info is ignored if it's not alongside actual documentation.

But still, it doesn't feels the best to me; because we include them right now, and would ignore them in the next version, this would be a breaking change. Also, I believe we can only include the type with numpy-style docstring, resulting into @type field being created with empty @param field:

>>> from pydoctor.napoleon.docstring import NumpyDocstring >>> d = NumpyDocstring(''' ... Args ... ---- ... a:str ... b:int ... ''') >>> str(d) '\n:param a:\n:type a: str\n:param b:\n:type b: int

What I'm saying is that:

Args ----- a:str b:int

Should be treated the same manner as

@type a:str @type b:int

What do you think?

because we include them right now, and would ignore them in the next version, this would be a breaking change

This to me is enough to justify it. So to confirm - Include docs if @type or @param present, otherwise no docs if neither present (regardless if type hinting). Sound right? I can definitely add the type origin on the desc there.

Include docs if @type or @param present, otherwise no docs if neither present (regardless if type hinting). Sound right?

Yes it sounds right, thanks for your work on this.

I've addressed this issue.

cretz · 2022-06-03T20:38:13Z

Will look/respond/fix probably early next week.

cretz · 2022-06-13T18:51:32Z

I apologize I have not been able to get back on this. I have been swamped with other tasks. I will try to return soon.

tristanlatr · 2022-07-01T19:20:40Z

Hi @cretz,

Do you think you will have time to finalize the work on this PR?
I can help by providing more test cases.

And if you don't have time in the end, I'll probably try to make it work anyway, because I believe this adjustments worth it.

But no rush anyhow,

Tell me what you think,

cretz · 2022-07-05T13:03:26Z

@tristanlatr - Sorry, I have been caught up getting our Python SDK going and have not been able to prioritize docs. I will try to revisit in a few days.

Update: Still hoping to revisit this soon time permitting.

cretz

@tristanlatr - I am now back available. Sorry for the hiatus. I have added response on the primary point about treating the two ways of annotating types differently. Once we land on a solution there, I'll fix conflicts and apply other needed changes.

pydoctor/test/test_epydoc2stan.py

cretz · 2022-07-18T15:51:59Z

pydoctor/epydoc2stan.py

+        # Only include parameter or return sections if any are documented
+        if any(p.body for p in self.parameter_descs):


We need a way to determine of the type info is coming from AST, in which case we should display it only if it's documented or if the type is coming from docstring fields, in which case it should display anyhow (with undocumented notes).

This seems a bit off. If someone has chosen to use @type (maybe to help their IDE or whatever) but have chosen not to use @param, they are choosing to not document, right?

It seems you are suggested that @type be treated as an empty @param doc? How would I, as a user that doesn't use @type, be able to get that empty documentation functionality the way those that do use @type get?

tristanlatr · 2022-08-25T16:17:03Z

Hi @cretz,

Do you think you’ll be able to work on this changes again ?

cretz · 2022-08-29T13:34:37Z

@tristanlatr - Sorry, this keeps getting pushed down on my priority list :-) It is still definitely on my TODO (this powers https://python.temporal.io/ for us), but cannot promise exact time.

…oming from the docstrings.

cretz · 2022-11-15T12:21:03Z

@tristanlatr - Thanks for helping this move forward! (pardon my absence)

…load

…load decorator.

… inconsistencies with the literal css class.

tristanlatr · 2022-11-16T20:15:26Z

Ok, we're getting there. This is ready :)

tristanlatr · 2022-11-16T20:14:17Z

pydoctor/epydoc2stan.py

+        # Only include parameter or return sections if any are documented
+        if any(p.body for p in self.parameter_descs):


I've addressed this issue.

tristanlatr · 2022-11-17T17:04:53Z

Any trepidation about this change in twisted docs ? @glyph
Personally, I think it's fine, but I'de like to hear it from you.

glyph · 2022-11-17T18:57:35Z

Any trepidation about this change in twisted docs ? @glyph
Personally, I think it's fine, but I'de like to hear it from you.

We don't use a lot of overloads yet, and as I understand it, despite the docs being arguably slightly "uglier" with this change, the ugliness is a reflection of real-world complexity that is currently just missing. So I would regard this as an improvement.

I think that having an option to toggle wrapping, specifically Black-compatible wrapping, would be good to have, but that is an unrelated problem for function signatures, so I don't think it has any bearing on this PR.

tristanlatr · 2022-11-17T19:19:14Z

Thanks for your feedback. I'll merge this PR :) Any option for line wrapping / disabling annotation in function signatures can be added later.

Function overload support

f07bdee

Replaces and closes twisted#417, fixes twisted#406

cretz force-pushed the overloads branch from 5b72df3 to f07bdee Compare June 2, 2022 13:24

tristanlatr mentioned this pull request Jun 2, 2022

@overload decorator support and add types annotations to signatures #417

Closed

cretz changed the title ~~Function overload support~~ Function overload support and all-param/return-undocumented change Jun 2, 2022

PR suggestions and remove params/returns if all undocumented

fc8c8da

cretz force-pushed the overloads branch from d1cda4d to fc8c8da Compare June 2, 2022 15:00

cretz marked this pull request as ready for review June 2, 2022 15:42

tristanlatr requested changes Jun 2, 2022

View reviewed changes

tristanlatr linked an issue Jun 2, 2022 that may be closed by this pull request

Display types in function signatures #473

Closed

Function overload updates

7cbfea3

cretz changed the title ~~Function overload support and all-param/return-undocumented change~~ Function overload support and param/return-sections-undocumented change Jun 3, 2022

tristanlatr reviewed Jun 3, 2022

View reviewed changes

pydoctor/templatewriter/pages/__init__.py Show resolved Hide resolved

tristanlatr reviewed Jun 3, 2022

View reviewed changes

tristanlatr requested changes Jun 3, 2022

View reviewed changes

cretz commented Jul 18, 2022

View reviewed changes

tristanlatr mentioned this pull request Aug 5, 2022

Better property support #640

Open

tristanlatr added 2 commits November 14, 2022 20:30

Merge commit '3c107745d2d2e1d190164e9a05c31d59d42a239e' into overloads

a60a307

Fix error

555c30d

tristanlatr and others added 10 commits November 15, 2022 00:07

fix mypy

4a20f85

Track fields origin and ensure we display type fields when they are c…

a739a43

…oming from the docstrings.

Apply suggestions from code review

8ce9a12

Also detect typing_extensions's version of overload.

f79b45f

Bump template version

d938f21

Add change log entry.

a7a373e

Ensure annotation are presented inside <code>, same for default values.

b66a438

Merge commit '8ce9a12e2fc687be85a79ed61a816ff3e258c4e3' into overloads

0618bc7

Add other entries in the changelog

1b9e07d

Fix pyflakes

0d3ab34

tristanlatr added 9 commits November 15, 2022 10:38

Use regex, less annoyance.

66a0a53

Use a slightly more compact presentation for signatures

2707cca

fix tests

64e37eb

Get the actual line numbers for the new warnings related to the @over…

483a482

…load decorator.

Use <code> tags for annotations but not for default values.

f80106f

remove unused import.

1570950

Use raw string to avoid inserting \n in docstring.

242d4df

Tweak readthedocs theme to render annotations better. Also fix little…

433d372

… inconsistencies with the literal css class.

Add changelog entry

2a1bb77

tristanlatr added this to the 22.x milestone Nov 16, 2022

tristanlatr approved these changes Nov 16, 2022

View reviewed changes

tristanlatr merged commit 3cb8476 into twisted:master Nov 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function overload support and param/return-sections-undocumented change #595

Function overload support and param/return-sections-undocumented change #595

cretz commented Jun 2, 2022 •

edited

Loading

tristanlatr commented Jun 2, 2022

cretz commented Jun 2, 2022 •

edited

Loading

codecov bot commented Jun 2, 2022 •

edited

Loading

tristanlatr left a comment

tristanlatr Jun 2, 2022

tristanlatr commented Jun 2, 2022

cretz commented Jun 2, 2022

cretz commented Jun 3, 2022

tristanlatr left a comment

tristanlatr left a comment

tristanlatr left a comment •

edited

Loading

tristanlatr Jun 3, 2022

tristanlatr Jun 3, 2022

tristanlatr Jun 17, 2022

cretz Jul 18, 2022

tristanlatr Jul 18, 2022

cretz Jul 18, 2022

tristanlatr Jul 18, 2022

tristanlatr Nov 16, 2022

cretz commented Jun 3, 2022

cretz commented Jun 13, 2022

tristanlatr commented Jul 1, 2022

cretz commented Jul 5, 2022 •

edited

Loading

cretz left a comment

cretz Jul 18, 2022

tristanlatr commented Aug 25, 2022

cretz commented Aug 29, 2022

cretz commented Nov 15, 2022

tristanlatr commented Nov 16, 2022

tristanlatr Nov 16, 2022

tristanlatr commented Nov 17, 2022

glyph commented Nov 17, 2022

tristanlatr commented Nov 17, 2022

		# Only include parameter or return sections if any are documented
		if any(p.body for p in self.parameter_descs):

Function overload support and param/return-sections-undocumented change #595

Function overload support and param/return-sections-undocumented change #595

Conversation

cretz commented Jun 2, 2022 • edited Loading

tristanlatr commented Jun 2, 2022

cretz commented Jun 2, 2022 • edited Loading

codecov bot commented Jun 2, 2022 • edited Loading

Codecov Report

tristanlatr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tristanlatr commented Jun 2, 2022

cretz commented Jun 2, 2022

cretz commented Jun 3, 2022

tristanlatr left a comment

Choose a reason for hiding this comment

tristanlatr left a comment

Choose a reason for hiding this comment

tristanlatr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz commented Jun 3, 2022

cretz commented Jun 13, 2022

tristanlatr commented Jul 1, 2022

cretz commented Jul 5, 2022 • edited Loading

cretz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tristanlatr commented Aug 25, 2022

cretz commented Aug 29, 2022

cretz commented Nov 15, 2022

tristanlatr commented Nov 16, 2022

Choose a reason for hiding this comment

tristanlatr commented Nov 17, 2022

glyph commented Nov 17, 2022

tristanlatr commented Nov 17, 2022

cretz commented Jun 2, 2022 •

edited

Loading

cretz commented Jun 2, 2022 •

edited

Loading

codecov bot commented Jun 2, 2022 •

edited

Loading

tristanlatr left a comment •

edited

Loading

cretz commented Jul 5, 2022 •

edited

Loading