Skip to content
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.

Sign up for GitHub

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

Discern property variables #277

Merged
merged 9 commits into from
Jun 22, 2024
Merged

Discern property variables #277

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b5fe6a7
Possible way to address issue 276.
johann-petrak Oct 21, 2020
8c2bb0d
Shorten comment line
johann-petrak Oct 21, 2020
8b2e439
Rename "vartype" to "kind".
johann-petrak Oct 28, 2020
450bcc3
Merge remote-tracking branch 'upstream/master' into issue276
johann-petrak Oct 28, 2020
c43c886
Remove unnecessary comment.
johann-petrak Oct 28, 2020
a5bf43b
Make linter happy about line length.
johann-petrak Oct 28, 2020
f4251dd
REF: Make pdoc.Doc.obj point to raw property/descriptor
kernc Oct 31, 2020
b87b43e
ENH: Introduce `Variable.kind: Literal["var","prop"]`
kernc Jun 22, 2024
7c67701
Merge remote-tracking branch 'upstream/master' into johann-petrak-iss…
kernc Jun 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions pdoc/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,12 +985,27 @@ def definition_order_index(
self.doc[name] = Function(
name, self.module, obj, cls=self)
else:
# check if the variable is really a property and if yes,
# if it does have a setter or deleter
# in which case it is not read-only
# dict(inspect.getmembers(clazz))[name]
kernc marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(obj, property):
kind = "property"
if obj.fget is not None:
kind += "/get"
Copy link
Member

@kernc kernc Oct 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think even /set and /del are superfluous; how much more marking a getter. 😆 Pdoc3, as I see it, is about covering the 95% use case. Over 95% of Python properties define a getter (conjecture; happy to see it disproved). I'd strongly prefer properties with just the getter have more plain kind='property'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just think that if a program automatically shows the properties of a property, then it should do so in a logical, consistent and correct way. For anyone who wants to use an API, it is important information if a property can be read/written/deleted and this would show all of that. The other approach would be to show none and force people to document manually (and probably forget to do that) what can be determined automatically. Doing half of it would be even odder, because then it would not be self-obvious what is shown and somebody would first have to figure it out, and implementors would have to still manually document the excluded cases.
I am not sure what harms the additions would do at all: not much space is taken up and what is shown should be immediately obvious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For anyone who wants to use an API, it is important information if a property can be read/written/deleted

True. The only thing better than extra information is an API that is designed so clearly and intuitively that it prompts no additional tagging or guesswork. Properties are properties, on-the-fly computed values. Most properties can be read, a few properties can be set (with encapsulation, evidently, the sole primary use case), and if a property can be deleted, by god, you had better explained it to me why in prose!

Apologies to your case, but I've thought about it a lot and reached the conclusion that I want Variable.kind be just plain 'property' for now, with potential additions subject to what Sphinx community decides and what pdoc community decides (i.e. by way of new issue and its upvotes). 😳

I have pushed a seemingly innocuous change (the tests pass 😅) that exposes raw property/descriptor objects (instead of merely their getters) in pdoc.Variable.obj. Thus, you can for the time being attach '-ro', '/get' (etc.) suffixes in your overridden html template by e.g.:

if isinstance(var.obj, property):
   if var.obj.fget:
      ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough! :) 👍

if obj.fset is not None:
kind += "/set"
if obj.fdel is not None:
kind += "/del"
else:
kind = "var"
self.doc[name] = Variable(
name, self.module,
docstring=(
var_docstrings.get(name) or
(inspect.isclass(obj) or _is_descriptor(obj)) and inspect.getdoc(obj)),
cls=self,
kind=kind,
obj=getattr(obj, 'fget', getattr(obj, '__get__', None)),
instance_var=(_is_descriptor(obj) or
name in getattr(self.obj, '__slots__', ())))
Expand Down Expand Up @@ -1466,10 +1481,10 @@ class Variable(Doc):
Representation of a variable's documentation. This includes
module, class, and instance variables.
"""
__slots__ = ('cls', 'instance_var')
__slots__ = ('cls', 'instance_var', 'kind')

def __init__(self, name, module, docstring, *,
obj=None, cls: Class = None, instance_var=False):
obj=None, cls: Class = None, instance_var=False, kind=None):
"""
Same as `pdoc.Doc`, except `cls` should be provided
as a `pdoc.Class` object when this is a class or instance
Expand All @@ -1489,6 +1504,12 @@ def __init__(self, name, module, docstring, *,
opposed to class variable).
"""

self.kind = kind
"""
One of `var`, `property/get/set/del` with one or more of get, set, del, e.g. property/get/set for
a property that can be read and set.
"""

@property
def qualname(self) -> str:
if self.cls:
Expand Down
2 changes: 1 addition & 1 deletion pdoc/templates/html.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@
<dl>
% for v in inst_vars:
<% return_type = get_annotation(v.type_annotation) %>
<dt id="${v.refname}"><code class="name">var ${ident(v.name)}${return_type}</code></dt>
<dt id="${v.refname}"><code class="name">${v.kind} ${ident(v.name)}${return_type}</code></dt>
<dd>${show_desc(v)}</dd>
% endfor
</dl>
Expand Down