sphinx-automodapi icon indicating copy to clipboard operation
sphinx-automodapi copied to clipboard

Incompatible with sphinx>4 (submodules of submodules)

Open pllim opened this issue 3 years ago • 17 comments

Not sure what is happening but we need to make sure it is not a bug here before reporting it upstream to sphinx. This is blocking astropy/astropy#11763 .

@larrybradley narrowed down the issue in https://github.com/astropy/astropy/issues/11723#issuecomment-838733176


I think this is a real issue. I'm getting this same error in photutils. It appears to be due to submodules of submodules:

mod1
    __init__.py  (from mod2 import *)
    mod2
       __init__.py (from a import *)
       a.py (defines myfunc, and __all__ = ['myfunc'])

In this case, Sphinx is now defining duplicate references as: package.mod1.myfunc and package.mod1.mod2.myfunc and raising the "duplicate object descriptions".

This started with Sphinx 4 release. Not sure if this was an unintended bug there or if something needs to be fixed in astropy-sphinx for compatiblity with Sphinx 4.

pllim avatar May 20 '21 19:05 pllim

I'll have a look at this the coming week ✋🏼

astrojuanlu avatar May 22 '21 08:05 astrojuanlu

🙏 Thank you! 🙏

pllim avatar May 22 '21 12:05 pllim

xref: https://github.com/sphinx-doc/sphinx/issues/9121 and https://github.com/sphinx-doc/sphinx/pull/9128/

astrojuanlu avatar May 24 '21 16:05 astrojuanlu

I'll close this after I confirm that the unpinning would work again. Thanks for the heads up!

pllim avatar May 24 '21 17:05 pllim

To clarify, this patch was released in v4.0.0b2, and it's present in all versions since then. @pllim I misled you into thinking that our bug https://github.com/astropy/sphinx-automodapi/issues/130 would be fixed by this, but it's the opposite: it's caused by it.

In the case of photutils, these are the affected classes: photutils.psf.matching.windows.CosineBellWindow, photutils.psf.matching.windows.HanningWindow, photutils.psf.matching.windows.SplitCosineBellWindow, photutils.psf.matching.windows.TopHatWindow, photutils.psf.matching.windows.TukeyWindow.

It's interesting that photutils.psf.matching.fourier functions do not seem to be affected. I added a photutils.psf.matching.fourier.TESTCLASS class, and this also triggered a warning. Not sure why classes and functions work differently on this.

Will add more information soon.

astrojuanlu avatar May 24 '21 17:05 astrojuanlu

@astrojuanlu , any update from the Sphinx side? If not, we might implement a workaround so we can remove the pin.

pllim avatar Jul 26 '21 21:07 pllim

Sorry I didn't have time to have a deeper look at this. I can do the week of August 9th, feel free to proceed with a workaround if you're in a hurry.

astrojuanlu avatar Jul 27 '21 22:07 astrojuanlu

No hurry (yet). Thanks for the update!

pllim avatar Jul 28 '21 13:07 pllim

See some analysis here https://github.com/astropy/photutils/pull/1238

astrojuanlu avatar Sep 07 '21 10:09 astrojuanlu

I have a minimal sphinx project producing the issue here:

https://github.com/maxnoe/sphinx-automodapi-duplicated-warning

maxnoe avatar Mar 29 '22 15:03 maxnoe

@eteq said he was gonna look at this... any update, Erik? Thanks!

If nothing is happening here, see astropy/photutils#1306 for downstream fix.

Related astropy PRs: astropy/astropy#12270 and astropy/astropy#13015

pllim avatar Mar 29 '22 20:03 pllim

So it seems we should somehow generate the autodoc templates using :canonical: for the classes etc. that are duplicated: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#directive-option-py-class-canonical (But I have no idea how to do that)

saimn avatar Apr 05 '22 16:04 saimn

This needs to go into the output of sphinx-automodapi somehow, which in turn seems to rely on sphinx-autodoc. Which doesn't seem to forward options to the class directive.

E.g. in ctapipe/docs/api I have three files for the same class cause it is imported in two __init__.py:

ctapipe.calib.camera.calibrator.CameraCalibrator.rst
ctapipe.calib.camera.CameraCalibrator.rst
ctapipe.calib.CameraCalibrator.rst

These are the files autogenerated by automod-api and contain rst instructions using sphinx.ext.autodoc: https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#directive-autoclass:

CameraCalibrator
================

.. currentmodule:: ctapipe.calib.camera

.. autoclass:: CameraCalibrator
   :show-inheritance:

   .. rubric:: Attributes Summary

   .. autosummary::

      ~CameraCalibrator.apply_peak_time_shift
      ~CameraCalibrator.apply_waveform_time_shift
      ~CameraCalibrator.data_volume_reducer_type
      ~CameraCalibrator.image_extractor_type

   .. rubric:: Methods Summary

   .. autosummary::

      ~CameraCalibrator.__call__

   .. rubric:: Attributes Documentation

   .. autoattribute:: apply_peak_time_shift
   .. autoattribute:: apply_waveform_time_shift
   .. autoattribute:: data_volume_reducer_type
   .. autoattribute:: image_extractor_type

   .. rubric:: Methods Documentation

   .. automethod:: __call__

So probably we should make try to get autodoc to include the :canonical: path of the class when it's run?

maxnoe avatar Apr 05 '22 16:04 maxnoe

autodoc has apparently a way to include that:

https://github.com/sphinx-doc/sphinx/blob/b4276edd848d112b4e981011c334d27cbcb20018/sphinx/ext/autodoc/init.py#L1674-L1676

Not sure how to trigger it though...

Commit adding this: https://github.com/sphinx-doc/sphinx/commit/acf66bc4d5b53189f893a50a235e710f063d629d

maxnoe avatar Apr 05 '22 16:04 maxnoe

The method you pointed @maxnoe (get_canonical_fullname) is correctly executed. But then it goes through

https://github.com/sphinx-doc/sphinx/blob/b4276edd848d112b4e981011c334d27cbcb20018/sphinx/domains/python.py#L1218-L1237

which triggers the warning.

If I take your reproduction project (very useful btw !), the generated docs seems correct. First we have a duplicate for duplicated.Foo:

.. py:class:: Foo()
   :module: duplicated
   :canonical: duplicated.foo.foo.Foo

   Bases: :py:class:`object`

   Awesome Foo class

And then another one for duplicated.foo.Foo (which is the ones triggering the warning):

.. py:class:: Foo()
   :module: duplicated.foo
   :canonical: duplicated.foo.foo.Foo

   Bases: :py:class:`object`

   Awesome Foo class

In the method I pointed above there is a dict with the duplicates:

(Pdb++) pp self.objects
{'duplicated.Foo': ObjectEntry(docname='api/duplicated.Foo', node_id='duplicated.Foo', objtype='class', aliased=False),
 'duplicated.foo.Foo': ObjectEntry(docname='api/duplicated.foo.Foo', node_id='duplicated.foo.Foo', objtype='class', aliased=False),
 'duplicated.foo.foo.Foo': ObjectEntry(docname='api/duplicated.Foo', node_id='duplicated.Foo', objtype='class', aliased=True)}

So I'm thinking that this may actually be a bug in Sphinx which correctly handles the case of one duplicate but not when there are more than one ?

saimn avatar Apr 11 '22 18:04 saimn

See https://github.com/sphinx-doc/sphinx/issues/10348#issuecomment-1100513041= for a very helpful and detailed description of the problem and the different options. Maybe we could add an option in automodapi to add :noindex: when necessary ?

saimn avatar Apr 19 '22 19:04 saimn

Reading the comment, I think a good solution (however I don't know how easy it would be) would be 2):

  1. Enable sphinx_automodapi to add the :noindex: option sometimes. This way, you can still use the extension but the newly documented objects are not added to the index, thus not exactly cross-referenceable and would not trigger any warning.

What do you think about the feasibility of adding a :no-index: option to .. automod-api that is a list of objects for which :no-index should be added?

E.g. for the minimal example you'd put:

.. automodapi:: duplicated.foo
   :no-index: duplicated.foo.Foo, duplicated.foo.Bar

?

maxnoe avatar Jun 13 '22 10:06 maxnoe