Skip to content

Show type annotations on properties#7385

Closed
eric-wieser wants to merge 1 commit intosphinx-doc:3.0.xfrom
eric-wieser:actual-property-annotation
Closed

Show type annotations on properties#7385
eric-wieser wants to merge 1 commit intosphinx-doc:3.0.xfrom
eric-wieser:actual-property-annotation

Conversation

@eric-wieser
Copy link
Copy Markdown
Contributor

@eric-wieser eric-wieser commented Mar 26, 2020

Fixes gh-7383

Keeping the description short since the issue gives the context.

An example in the wild:

image

@eric-wieser eric-wieser force-pushed the actual-property-annotation branch 4 times, most recently from 65d2e46 to d865626 Compare March 27, 2020 12:19
Note that in order to do this, we extend the signature parser to allow things like:

```rst
.. attr:: MyClass.attr : int
    docs
```

which mirrors the syntax of

```python
class MyClass:
    attr : int
```
@eric-wieser eric-wieser force-pushed the actual-property-annotation branch from d865626 to 813d515 Compare March 27, 2020 13:02
@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Mar 27, 2020

Thank you for proposal. Seems good to me. But, sadly, we have a plan to separate property to an independent directive (see #7068) in 3.1.0. So I'd not like to merge this into 3.0.

@eric-wieser
Copy link
Copy Markdown
Contributor Author

eric-wieser commented Mar 27, 2020

What are your thoughts on the part of this patch which allows:

.. attr:: MyClass.attr : int
    docs

for consistency with

.. method:: MyClass.meth() -> int
    docs

I could put that part in separately.

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Mar 27, 2020

Since 2.4.0, we've provided :type: option to describe the type of an attribute:

.. attr:: MyClass.attr
   :type: int

    docs

So that is already implemented in different way.

@eric-wieser
Copy link
Copy Markdown
Contributor Author

eric-wieser commented Mar 27, 2020

I'm aware it's already possible to provide this data to sphinx.

My suggestion is more about matching python syntax. Due to python 3.5 supporting:

def meth() -> int:
    pass

you added support for writing

.. method:: meth() -> int

as a short version of

.. method:: meth()
    :rtype: int

Python 3.6+ now supports:

some_var : int

So by symmetry, I think it makes sense to support

.. data:: some_var : int

as a short version of

.. data:: some_var 
    :type: int

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Mar 27, 2020

Okay, I understand.

Before starting the discussion, I need to correct your misunderstanding. :rtype: is not a short version of -> type. It's a part of content of directive (it's a field-list!), not an option of the directive.

.. py:function:: hello(name)

   blah blah blah

   :param name: the name of user who logs in
   :type name: str
   :rtype: str

It was added before type annotations support in python. So it does not modify the signature of function in the generated document. Surely they are similar, but different notation.

On the other hand, :type: is an option of the directive to annotate the data members and attributes. So it will be conflicted with the : type annotation if adopted. So I think :type: option would be deprecated if we adopted a new style.

The reason why I added :type: option instead of : type notation is the existence of its value. So far, we use :annotation: option for a long to describe the value of the data member:

.. py:data:: name
   :annotation: = 'tk0miya'

This declaration will be converted to `name = 'tk0miya'`.

On development 2.4, I separated the :annotation: option to two options; :type: and :value:. At that time, I thought to give python style signature to the argument (ex. .. py:data:: name: str = 'tk0miya') once. But I thought that passing them as directive options is simpler than passing them as a python-like string. To be precise, I hesitate to build and parse a python-like string on two modules. In introspect, the python way is easy to understand for users.

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Mar 27, 2020

Anyway, 3.0.x branch is now under feature freeze for the coming release. No more breaking changes and deprecations are prohibited. So it would be better to change these ideas on 3.x branch.

@tk0miya tk0miya added this to the 3.1.0 milestone Mar 27, 2020
@eric-wieser
Copy link
Copy Markdown
Contributor Author

Agreed

Okay, I understand.

Before starting the discussion, I need to correct your misunderstanding. :rtype: is not a short version of -> type. It's a part of content of directive (it's a field-list!), not an option of the directive.

.. py:function:: hello(name)

   blah blah blah

   :param name: the name of user who logs in
   :type name: str
   :rtype: str

It was added before type annotations support in python. So it does not modify the signature of function in the generated document. Surely they are similar, but different notation.

On the other hand, :type: is an option of the directive to annotate the data members and attributes. So it will be conflicted with the : type annotation if adopted. So I think :type: option would be deprecated if we adopted a new style.

Thanks for the explanation, that makes sense to me

The reason why I added :type: option instead of : type notation is the existence of its value. So far, we use :annotation: option for a long to describe the value of the data member:

.. py:data:: name
   :annotation: = 'tk0miya'

This declaration will be converted to `name = 'tk0miya'`.

On development 2.4, I separated the :annotation: option to two options; :type: and :value:. At that time, I thought to give python style signature to the argument (ex. .. py:data:: name: str = 'tk0miya') once.

I think what I actually care about most is that when viewing the web docs, I'd prefer to see the documentation in the form

name : str = 'tk0miya'

instead of

name = 'tk0miya'
    type: str

I suppose this is orthogonal though, and there's no need to make the rst match.

But I thought that passing them as directive options is simpler than passing them as a python-like string. To be precise, I hesitate to build and parse a python-like string on two modules. In introspect, the python way is easy to understand for users.

I'd argue it's (slightly) confusing to the user that they can write their method docs using python syntax, but have to fall back to directive options to build method docs.

Perhaps the hesistation would go away if ast.parse were used to parse the python string instead of regex?

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Mar 28, 2020

I think what I actually care about most is that when viewing the web docs, I'd prefer to see the documentation in the form

As you said, this is not related to the notations. In fact, the current python domain generates such output.

.. py:data:: name
   :type: str
   :value: 'tk0miya'

スクリーンショット 2020-03-28 12 32 14

I'd argue it's (slightly) confusing to the user that they can write their method docs using python syntax, but have to fall back to directive options to build method docs.

What do you mean? I guess nobody confuses by method notation. And there no directive options to describe the type of method.

Perhaps the hesistation would go away if ast.parse were used to parse the python string instead of regex?

Yes, acceptable.

@eric-wieser
Copy link
Copy Markdown
Contributor Author

Thank you for proposal. Seems good to me. But, sadly, we have a plan to separate property to an independent directive (see #7068) in 3.1.0. So I'd not like to merge this into 3.0.

What is the status of this plan? All the referenced PRs appear to be closed without merging.

@eric-wieser
Copy link
Copy Markdown
Contributor Author

I think what I actually care about most is that when viewing the web docs, I'd prefer to see the documentation in the form

As you said, this is not related to the notations. In fact, the current python domain generates such output.

.. py:data:: name
   :type: str
   :value: 'tk0miya'
スクリーンショット 2020-03-28 12 32 14

Is this limited to data? I can't seem to get it to work for attr.

@eric-wieser
Copy link
Copy Markdown
Contributor Author

eric-wieser commented Apr 27, 2020

Ping, @tk0miya - could you respond to the above comment?

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Apr 27, 2020

What is the status of this plan? All the referenced PRs appear to be closed without merging.

Now I reopened #7298. I'll work for it in this week. Please wait for a moment.

Is this limited to data? I can't seem to get it to work for attr.

Really? It seems working fine on my local:

.. py:attribute:: foo
   :type: str
   :value: 'tk0miya'

@eric-wieser
Copy link
Copy Markdown
Contributor Author

Really? It seems working fine on my local

Yep, works fine for me too now - I can only assume the CI i was using had a different sphinx version to what I thought. Sorry for wasting your time!

@tk0miya tk0miya modified the milestones: 3.1.0, 3.2.0 May 23, 2020
@tk0miya tk0miya closed this Jun 8, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants