Regression with 1.5.2 for classes where the base class is not included

[funkyfuture]

when i built a previously working documentation with version 1.5.2 Sphinx throws a bunch of error messages like:

None:1: WARNING: autosummary: failed to import _parserlib.utils._happy_contextmanager.<locals>.decorator.<locals>.wrapper.
Possible hints:
* ModuleNotFoundError: No module named 'parserlib._parserlib'
* AttributeError: type object 'TokenClass' has no attribute '_parserlib'
* AttributeError: 'function' object has no attribute '<locals>'
* ModuleNotFoundError: No module named 'parserlib.TokenClass'
* AttributeError: module 'parserlib' has no attribute '_parserlib'
* ModuleNotFoundError: No module named '_parserlib.utils._happy_contextmanager'; '_parserlib.utils' is not a package

that doesn't occur when i check out ba868daf4fecc72bd1a8957c565760d4c41eb71a.

i assume @haiiliin's change is targeting a use case where a base class is included in the docs. that's not applicable in my case. i think both cases are legit and could even occur in the same documentation. unless i'm missing something, this should be addressed by introducing a configuration option. i didn't come up with any idea how this could look like.

[kalekundert]

Thanks for making a report, and it makes sense that #22 might have added some requirement that the base class be in the docs. I don't really understand the error message you posted. I assume that parserlib, TokenClass, happy_contextmanager, etc. are all part of the project being documented. I don't know what the class and base class in question are, though.

A per-directive config option would be the most straight-forward way to handle this, but it's not clear how users could be expected to set this option to something reasonable when using autosummary.

That said, the only real alternative would be to hook into sphinx, keep track of all the classes that are documented, and then alter how the cross-references are generated based on that. That sound really complicated, though.

Also, I just noticed #31, which makes some more changes in this area. I have to take a look at that, although I don't think it will address this issue.

[funkyfuture]

I don't really understand the error message you posted.

me neither and they are certainly generated after autoclasstoc's transformations by another part of the process.

I assume that parserlib, TokenClass, happy_contextmanager, etc. are all part of the project being documented.I don't know what the class and base class in question are, though.

yes these object belong to the project, but none of it is to be included in the docs. it's all messed up, for example TokenClass is a member of the _parserlib module, so the latter can't be missing from the former.

I am working on developing an extension to reference multiple Python targets in haiiliin/sphinx-autodoc-toolbox#2

is this project intended as extension or alternative to this one?

the following syntax will be supported:

:py:class:`example.Parent|example.Unknown`

i'm not sure whether i get it. this is a reference, but would it also work as .. autoclass: example.Parent|example.Unknown? have you considered alternatives to |, which might be interpreted as boolean or? maybe -> or just >? but generally i find the idea clear and obvious to use. at least when i understood correctly: render the docs for example.Unknown and include its inherited memebrs of example.Parent.

one thing that i could provide soonish: shall i set up a logic for integrations tests, so that i can add a set of concrete input data? so that such regressions don't go unnoticed.

[haiiliin]

I just figure out the solution for cross reference on multiple targets in autosummary, you can check it here, it can be a solution to this issue.

i'm not sure whether i get it. this is a reference, but would it also work as .. autoclass: example.Parent|example.Unknown? have you considered alternatives to |, which might be interpreted as boolean or? maybe -> or just >? but generally i find the idea clear and obvious to use. at least when i understood correctly: render the docs for example.Unknown and include its inherited memebrs of example.Parent.

It works on :py:obj: roles, I am still working on the autodoc directives. I think multiple targets are equal to each other, so I would prefer using |, or maybe ,, maybe or is also a good choice.

[haiiliin]

which might be interpreted as boolean or?

I think the concept is very close to or, just like cls = None or Parent in python.

[kalekundert]

Integration tests are definitely needed, but I have some strong views on how they should be done, so it's probably best if I write the initial implementation. I'll do that this weekend, and update you if I don't manage to finish.

The py:class:``ref1|ref2`` syntax seems like a cool feature, but I don't understand how it would solve this problem. My understanding is that the point of #22 is so that child classes don't need to include :inherited-members: anymore, because the links will go directly to the parent class. But if we need to include a "backup" link to the child class itself, then the class still needs to use :inherited-members:, and we're still in pretty much the same situation we started in.

Maybe it would be a good idea to make a custom cross-reference type for the links that autoclasstoc makes. This could be similar to what you did in sphinx-autodoc-toolbox, expect that when a reference couldn't be found, it would just not make a link instead of trying a second reference. That would leave no way to get documentation for methods of undocumented base classes, but if that's a problem for anyone, the obvious solution would be to document said base classes.

[haiiliin]

The syntax Parent.method|Child.method in autosummary to generate the autoclasstoc table could be a way to find the link to the method automatically, by default, it links to Parent.method if it exists, otherwise it links to Child.method.

If we want to specify which target should be linked explicitly, we could add a config variable, in conf.py (the default behavior) or in the directive options of autoclastoc (specific behaviors in some places).

For example, we could add a config variable link_to_parent, it defaults to True, in the autosummary table, we could use something like Parent.method|Child.method, otherwise use Child.method|Parent.method. This option can be override in the options of the autoclasstoc directive, like:

.. autoclasstoc::
    :link_to_parent:

But if we need to include a "backup" link to the child class itself, then the class still needs to use :inherited-members:, and we're still in pretty much the same situation we started in.

If we want to link to the child method, we would definitely need :inherited-members:, using the above methods, if :inherited-members: is not included, it will try to link to the parent method.

If we always want to link to the child method, we could set link_to_parent to False and include :inherited-members:.

[kalekundert]

Here's the case I was thinking of:

class Parent:
    def method():
        pass

class Child(Parent):
    pass

In this case, there won't be a Child.method to link to unless the child class was documented with :inherited-members:, which we want to avoid. So I don't see how the Parent.method|Child.method syntax would help here.

[haiiliin]

Here's the case I was thinking of:

class Parent:
    def method():
        pass

class Child(Parent):
    pass

In this case, there won't be a Child.method to link to unless the child class was documented with :inherited-members:, which we want to avoid. So I don't see how the Parent.method|Child.method syntax would help here.

In this way Parent.method|Child.method will link to Parent.method, if Parent is not included in the documentation and :inherited-members: is not included, it fails to find the target, I think this is something that the users must guarantee to avoid by themselves.

In my point of view, if the user explicitly links the method to the method of Parent or Child, he should guarantee the method is included in the documentation.

[kalekundert]

I added an integration test framework yesterday. The next step is to come up with a test case that reproduces this regression, because as it stands now, I have no idea what the problem is. Realistically I won't have much time to look at this until next week, but I'd be happy to merge new test cases if anyone feels inclined to write them.

[kalekundert]

I think I figured out what was causing this regression. It's not that the parent class isn't included in the documentation, it's that the parent class can't be imported by autodoc. This scenario is now covered by the test_autoclasstoc[link-parent-method-cant-import] test case, which experiences the same regression and gives a similar (though not identical) error message to what's described above.

With this in mind, the solution was to check if the parent class could be imported before making links to it and if it couldn't, to leave the qualified module/class name out of the generated RST. I don't believe that any new config options are needed.

Let me know if this doesn't actually solve your problem, or if it inadvertently creates new problems.

[funkyfuture]

it works for me, thanks!