-
Notifications
You must be signed in to change notification settings - Fork 56
Handling of attrs and dataclass constructors #656
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
base: master
Are you sure you want to change the base?
Conversation
…tected constructor Function.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #656 +/- ##
==========================================
- Coverage 93.18% 93.15% -0.04%
==========================================
Files 47 49 +2
Lines 8736 9024 +288
Branches 1601 1667 +66
==========================================
+ Hits 8141 8406 +265
- Misses 336 349 +13
- Partials 259 269 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ydoctor into 718-dataclass-like-abstraction
Include the factory call as default value when possible.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Uses a converter | ||
""" | ||
|
||
Pydoctor also supports the newer APIs (``attrs.define``/``attrs.field``). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pydoctor also supports the newer APIs (``attrs.define``/``attrs.field``). | |
Pydoctor also supports the newer APIs (``attrs.define``/``attrs.field``); as well as ``dataclasses.dataclass`` generated classes. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Diff from pydoctor_primer, showing the effect of this PR on open source code: sdk-python (https://github.com/temporalio/sdk-python): typechecking got 1.12x slower (200.4s -> 224.2s)
(Performance measurements are based on a single noisy sample)
+ /projects/sdk-python/temporalio/client.py:2967: ambiguous ref to typed_search_attributes, could be temporalio.client.WorkflowExecution.typed_search_attributes, temporalio.client.ScheduleActionStartWorkflow.typed_search_attributes, temporalio.client.ScheduleDescription.typed_search_attributes, temporalio.client.ScheduleListDescription.typed_search_attributes
+ /projects/sdk-python/temporalio/client.py:2967: Cannot find link target for "typed_search_attributes"
+ /projects/sdk-python/temporalio/worker/_tuning.py:67: Cannot find link target for "CompositeTuner"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the generated docstring could be a bit better, but otherwise this seems great. No need to think super hard about it though, if you can think of a slightly better boilerplate we can improve it more later.
list_of_numbers: List[int] = list(), | ||
converted_paths: List[str] = list()): | ||
""" | ||
attrs generated method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be a note like @note:
, with a title line "Initialize a SomeClass"? In the summary, I think seeing "attrs generated method" on every __init__
is even less helpful than that.
Hi @glyph and many thanks for your review as always. I will move the words saying whether it’s a attrs or dataclass generated method in a I’ll try to improve one or two aspect still, namely
|
This PR drastically improve our understanding of attrs and dataclass generated classes.
It will infer the
__init__
signature and present fields docstrings in the parameter table.It understand combinations of auto_attribs, auto_detect, kw_only and init arguments to the attr.s() function. As well as the new APIs attrs.define(). For attr.ib(), it checks the converter, factory, default, type, kw_only and init arguments to infer each constructor param.
It only handles the creation of the
__init__
method, it’s does not create__eq__
and other attrs methods. Since these methods all have the same signature, it’s probably not worth it to actually create Function instance to represent them. One thing we could do it to add a note at the end of the docstring listing implemented methods. Another thing to do would be to show the class decorators so the reader can determine whether all compare methods are implemented.See documentation here: https://pydoctor--656.org.readthedocs.build/en/656/codedoc.html#using-attrs
Partial Fix for #305, handles the dataclass and attrs case.
Fixes #890
Currently, It will sometimes generate a wrong constructor arguments order for dataclasses and classic attrs based classes that have multiple inheritance and attribute collisions. This is because there are two manner of linearizing the parameters and I only implemented the “collect by MRO” order.