sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

autodoc-skip-member "what" value is wrong, and "name" is not fully qualified but doc says it is

Open ben-spiller opened this issue 4 years ago • 13 comments

Describe the bug If I document a class X with a method y() on it, when autodoc-skip-member is called for the method y() the value of what="class" and name="y". The doc says name is the fully qualified name (which would be X.y) - see http://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html It also says "what" refers to the item being documented (not what the parent object is) - so in this case it should be "method" not "class".

e.g. from logging "what" and "name" values: autodoc_skip_member: what=class, name=module, obj=mymodule autodoc_skip_member: what=class, name=repr, obj=<function MyClass.repr at 0x000001A600040D08>

etc

To Reproduce See above

Expected behavior The passed in "name" should be qualified (as documneted) i.e. include the package and (if appropriate) class, since a typical implementation might be to look up a blacklist/whitelist of names in a set or dict and this is much more difficult to implement if user needs to do some hacky logic to try to get the qualified name from the obj.

The "what" value for a method should be "method" not "class"

Environment info

  • OS: Windows
  • Python version: 3.6
  • Sphinx version: 2.2.0
  • Sphinx extensions: sphinx.ext.autodoc, sphinx.ext.autosummary

ben-spiller avatar Nov 10 '19 23:11 ben-spiller

I have also experienced this bug, just this week. Even if obj is a function, what says it is a class, or (less often) a module. The documentation is wrong.

Using Running Sphinx v3.2.0 on Mac, with Python 3.6.10 on Anaconda .

mattagape avatar Aug 12 '20 13:08 mattagape

+0 for changing the behavior. But we have to consider how to keep the compatibility of this behavior because this bug was implemented as its beginning.

tk0miya avatar Aug 12 '20 13:08 tk0miya

This is also wrong for attributes. In the case of attributes, what is "class", name is just the plain name of the attribute and obj is actually sphinx.ext.autodoc.INSTANCEATTR.

I think a new API should be added which can also fix the deficiency that when processing an attribute you don't know which class it belongs to.

Along the lines of

def autodoc-skip-member-v2(app, what, name, obj, parent, skip, options)

Where what is actually correct. obj being the object itself, which would be the value of an attribute for what=attribute (consistent with autodoc-process-docstring) and parent would be the class it was found in for attributes, methods etc. and None otherwise

enkore avatar Feb 15 '21 15:02 enkore

Hi, I am also experiencing this bug. I'm trying to prevent sphinx from generating documentation for an inherited static class in Python but the project's documentation and .rst files are generated so I'm trying to do it through conf.py using autodoc-skip-member. Unfortunately, because the name states only the method name and not the class of that method, and the obj states only the base class and not the child class, I can't discern between the base class method and the method inherited by the child class. This information is available in autodoc-process-signature, however, but it seems that autodoc-process-signature runs after autodoc-skip-member so I can't use the two methods in tandem to skip the inherited method. If anyone has any suggestions on how to work around this issue, please let me know!

TrentDaniel avatar Apr 21 '21 04:04 TrentDaniel

Hi @TrentDaniel we had a very similar problem. We got round it by using Python's inspect module. I'll ask if we can paste the relevant code here.

mattagape avatar Apr 21 '21 09:04 mattagape

Hi @mattgillucl, I understand if you can't post your own code for confidentiality reasons, but I would really appreciate it if you could even just tell me the basic logic of your workaround or which attributes you targeted through the inspect module. :)

TrentDaniel avatar Apr 21 '21 21:04 TrentDaniel

def which_functions_to_print(clazz):
    """
    Which functions do we want to print?
    :param clazz: class object under consideration
    :return: returns a list of function objects we want to print
    Written by Asif
    """
    # get all the functions in this class
    class_functions = dict(inspect.getmembers(clazz,
                                              predicate=inspect.isfunction))

    ok_to_print = []

    # for each function in this class
    for func_name, func_obj in class_functions.items():
        # for func in func_list:
        # func_name, func_obj, _ = func
        # import pdb; pdb.set_trace()
        should_i_print = True

        # for each base class of this class
        for baseclass in clazz.__mro__:
            # skip over the class we're checking
            if baseclass != clazz:
                # get the functions of the base class
                functions_base_class = dict(
                    inspect.getmembers(baseclass,
                                       predicate=inspect.isfunction))

                # if there is a function with the same name as base class
                if func_name in functions_base_class:
                    # if the function object is the same
                    # as one defined in a base class
                    if func_obj == functions_base_class[func_name]:
                        # print(f'{func_name} in subclass is same as function in'
                        #       f'{baseclass.__name__} (not overridden)')
                        should_i_print = False
                        break
                    else:
                        # print(f'{func_name} in subclass is not the same '
                        #       f'as one in baseclass {baseclass.__name__}')
                        pass
                else:
                    # print(f'{func_name} is not in '
                    #       f'baseclass {baseclass.__name__}')
                    pass

        if should_i_print:
            # print(f'\t✓✓✓ {func_name} is implemented in the subclass - print ')
            ok_to_print.append(func_obj)
        else:
            # print(f'\txxx {func_name} has been inherited from a subclass'
            #       f'- do not print')
            pass

    return ok_to_print

mattagape avatar Apr 22 '21 08:04 mattagape

@mattgillucl, thanks! I'll try this out!

TrentDaniel avatar Apr 22 '21 13:04 TrentDaniel

To summarize, there are two problems with autodoc-skip-member.

(a). The values of what and name are wrong. (b). It has been wrong as its beginning, so many code uses this event.

Therefore, in order to fix the problem "(a)" and keep backward compatibility "(b)", we need to prepare another event. The idea of autodoc-skip-member-v2 sounds good, but we need to consider whether it should be named differently from autodoc-skip-member. I also thought that the rules we decided here would be the ground rules for the future.

@tk0miya and I discussed this issue in today's hack-a-thon, and we came up with the following.

  • The name of event would not be changed for each versions (because separate event names would complicate the issue of call order, as described below)
  • Make the event have a mechanism to specify the version. For example, specify the version value as a separate argument, or use a colon and the version value, like autodoc-skip-member:2.
  • The version value should be only a number like 2. There is a possibility that non-numeric values such as v2 or 2.1 will be allowed in the future.
  • Version numbers should not be ordered; functions to be connected should have different signatures for each version, so a mechanism to specify a range of version values is not required.
  • Extension implementors that uses connect can implement extensions work with older Sphinx versions. If an ExtensionError is raised when connecting to a newer version of an event, fall back to the older version.
  • The order in which connected functions are called will be kept in the priority order, and if they have the same priority, they will be called in the order in which they were connected "regardless of the version of the event".
  • Extension implementors that uses emit and support multiple version of event must change the emit function call to convey the event to the connected functions of mixed versions at once.

From the above, autodoc-skip-member event will look like this.

app.add_event('autodoc-skip-member')  # for unnamed version
app.add_event('autodoc-skip-member', version=2)  # for version 2
app.connect('autodoc-skip-member', func)  # for unnamed version
app.connect('autodoc-skip-member', func, version=2)  # for version 2
app.emit('autodoc-skip-member', ...)  # emit to all versions at the once...

or

app.add_event('autodoc-skip-member')  # for unnamed version
app.add_event('autodoc-skip-member:2')  # for version 2
app.connect('autodoc-skip-member', func)  # for unnamed version
app.connect('autodoc-skip-member:2', func)  # for version 2
app.emit('autodoc-skip-member', ...)  # emit to all versions at the once...

shimizukawa avatar May 01 '21 08:05 shimizukawa

Hi @shimizukawa, I'm not sure I got the point behind the following texts:

"Extension implementors that uses emit and support multiple version of event must change the emit function call to convey the event to the connected functions of mixed versions at once"

and the single line in the examples:

"# emit to all versions at the once..."

In as much as the various versions of an event have different signatures (usually defined with positional arguments), how could it be possible that the arguments passed to the emit() by the emitter call would fulfill all versions of handler signatures?

Please forgive my misunderstanding if so. :) In case I'm not completely wrong, I'm going through my suggestion behind.


In order to prepare "ground rules for the future", I've got the feeling that the idea of defining event handlers with positional argument functions should be challenged.

Considering that, I'm thinking of the following alternatives:

  1. Use keyword argument handlers,
  2. Define an object representing the event.

Solution 1: keyword arguments

With keyword arguments, new versions of an event could add extra parameters without causing an error on previous versions of handlers registered.

def my_autodoc_skipmember(app, what, name, obj, skip, options, **kwargs):
    pass
app.connect("autodoc-skip-member", my_autodoc_skipmember)
app.emit('autodoc-skip-member', what=..., name=..., obj=..., skip=..., options=..., new_arg1=...)

The pitfalls I see behind this solution are:

  • the client handlers MUST include the **kwargs pattern in their signature, otherwise future versions of the event would cause an error on them,
  • the client handlers MUST name their parameters the way they are passed on by the emitter,
  • the transition from positional argument to keyword argument style seems messy and could lead to unpredictable errors.

Solution 2: event objects

This solution describes a new API around events (and emitters and handlers by the way). Events could be described with dedicated objects, passed as a whole to the event handlers.

def my_autodoc_skipmember(app, event):
    pass
app.connect2("autodoc-skip-member", my_autodoc_skipmember)
app.emit2("autodoc-skip-member", SkipMemberEvent(what=..., name=..., obj=..., skip=..., options=..., new_arg1=...))

Doing so, a new version of the event could add new member variables in the events, without requiring the handler signature to change. The backward compatibility with the former positional argument handlers could easily be handled.

This solution seems preferrable to me.

alxroyer avatar May 03 '21 08:05 alxroyer

In as much as the various versions of an event have different signatures (usually defined with positional arguments), how could it be possible that the arguments passed to the emit() by the emitter call would fulfill all versions of handler signatures?

You are right. I've discussed the emit part with tk0miya, but we don't have any definite ideas at the moment. For example, we had an idea that we could pass all the different parameters for each version.

app.emit('autodoc-skip-member', app, what, name, obj, skip, options)  # v1 only
app.emit('autodoc-skip-member', {
    1: (app, what, name, obj, skip, options),
    2: {kw1: value1, kw2: value2, ...},
    3: {kw1: value1, kw2: value2, kw3: value3, ...},
})  # multiply versions

Solution 1: keyword arguments

I think the problem with Solution 1 is that not only the necessary arguments are passed to the event handler side, but all the arguments of emit will be passed. Therefore, existing event handler implementations that are passed new keyword arguments will not work.

Solution 2: event objects

The idea of Solution2 seems to be good for event handlers that are compatible with newer versions. However, existing (v1) event handlers will not be able to handle SkipMemberEvent, so it will be necessary to replace the traditional positioning arguments in some way and call the v1 event handler. For example, a method that converts the arguments to v1 style can be added to the SkipMemberEvent class. This seems better than the "list all the arguments set for each version in a dictionary" method I mentioned.

shimizukawa avatar May 08 '21 21:05 shimizukawa

Great. Looking forward to see it go further. Yours,

alxroyer avatar May 09 '21 08:05 alxroyer

Just wondering if it is still being worked on as I am encountering the same issue?

jazzblue avatar Aug 10 '22 16:08 jazzblue

Thanks for the suggestion. I'll close it.

shimizukawa avatar Dec 18 '22 06:12 shimizukawa