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.

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

py domain: All :py:* roles can refer python objects even if objtype not matched. #7243

Closed
tk0miya opened this issue Mar 3, 2020 · 4 comments

Comments

@tk0miya
Copy link
Member

tk0miya commented Mar 3, 2020

Describe the bug
py domain: All :py:* roles can refer python objects even if objtype not matched.

To Reproduce

Both :py:meth: and :py:class: are converted into hyperlinks to .. py:function:: foo.

.. py:function:: foo

:py:meth:`foo`
:py:class:`foo`

Expected behavior
They fails to create hyperlinks for python object when objtype is different.

Your project
N/A

Screenshots
N/A

Environment info

  • OS: Mac
  • Python version: 3.8.2
  • Sphinx version: 2.4.4
  • Sphinx extensions: N/A
@tk0miya tk0miya added this to the 3.0.0 milestone Mar 3, 2020
tk0miya added a commit to tk0miya/sphinx that referenced this issue Mar 3, 2020
…hing obj

All py roles does not check objtypes on searching. So they can
refer all kind of python objects even if objtype not matched.
tk0miya added a commit to tk0miya/sphinx that referenced this issue Mar 5, 2020
…doc#7243)

All :c:*: roles can refer all kind of C objects even if objtype
not matched.  To search objects correctly, this change the form of
key for objects to a pair of objtype and objname.
tk0miya added a commit that referenced this issue Mar 5, 2020
All :c:*: roles can refer all kind of C objects even if objtype
not matched.  To search objects correctly, this change the form of
key for objects to a pair of objtype and objname.
@tk0miya
Copy link
Member Author

tk0miya commented Mar 5, 2020

I found same problem in C domain:

.. c:function:: hello()

:c:member:`hello`
:c:macro:`hello`
:c:func:`hello`

tk0miya added a commit that referenced this issue Mar 5, 2020
All :c:*: roles can refer all kind of C objects even if objtype
not matched.  To search objects correctly, this change the form of
key for objects to a pair of objtype and objname.
@jakobandersen
Copy link
Contributor

First: @tk0miya, thanks for all the work and maintenance! It's really nice.

I'll look more closely at the C domain soon(TM), but just a few semi-relevant comments for now.

This is definitely an issue that should be resolve in each domain, though I'm not entirely sure it should be solved in the same style everywhere, nor that it shouldn't be. Different languages/domains may have different semantics that begs for different resolutions.
In general, for the programming language domains, I believe that xref roles should resolve almost* independent of object types. Once resolved, then if there is a mismatch of role type and object type, a warning should be issued.
E.g., for the example:

.. py:function:: foo

:py:meth:`foo`, great, role type matches the object it resolves to
:py:class:`foo`, warning, target ``foo`` is a function but the xref role is a class

That is, the second xref becomes the exact same link** as the first. This is the scheme I implemented in the C++ domain.
From a brief view, PR #7256 implements the alternative where the second link triggers a warning about foo not found, which I personally find misleading.***

* Different roles may parse their argument with differently. E.g., the cpp:func role allows for specifying a specific function overload. This parsing option is not enabled in the other roles.
** Different roles may transform the link text in different ways. Notably, the function roles in multiple (all?) domains has the fix_parens option enabled, which ensures that the link text ends with (), e.g., :py:meth:`foo` becomes a link with text foo().
*** When time allows I can make the necessary fixes in the C domain, though it may be some more weeks before I get to it.

@tk0miya
Copy link
Member Author

tk0miya commented Mar 6, 2020

Agreed. I don't think all domains are fixed as same. Now I wonder this fix is really helpful to Python. For example, we have a discussion about attributes and properties (refs: #7068). They are different in implementation, but behave almost same. Other case, we received a post about typing.Any in sphinx-users form. Usually it is used like a type. But it is a variable, not a class.

In python, everything is an object. Variables, functions and classes are all objects. And we don't need to know the some object is either a function, a class or an instance. For example, int is a class. But somebody might consider it as a function which converts string to integer like int("1"). I also did not know staticmethod is a class, not a function (decorator). In my understanding, the behavior of the objects is determined by special methods (dunder methods). So all instances, functions, classes and other all objects having __call__() methods behave like "callable".

Therefore, my proposal in this issue would be correct. But it would also be uncomfortable restriction for python developers. I reconsider again what we should do for python domain especially.

Note: On the other hand, I feel no problem for C domain to fix this.

tk0miya added a commit to tk0miya/sphinx that referenced this issue Mar 6, 2020
…hing obj

All py roles does not check objtypes on searching. So they can
refer all kind of python objects even if objtype not matched.
@tk0miya
Copy link
Member Author

tk0miya commented Mar 8, 2020

Closing this now. I think this will harm users.
Thanks,

@tk0miya tk0miya closed this as completed Mar 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

No branches or pull requests

2 participants