Added test case for importing a class from a private submodule into a public one

[astrofrog]

Investigating the issue from astropy/astropy#16974

[astrofrog]

For now this just adds the regression test. One way to solve that specific use case is to do:

.. automodapi:: sphinx_automodapi.tests.example_module.public
    :allowed-package-names: sphinx_automodapi.tests.example_module._private

however this could get tedious if we ever did use this pattern of private submodules extensively in a package. I wonder if actually what we want to do is to have an option to respect __all__ if it is present, as the authoritative version of what should be documented in a module.

[astrofrog]

I think I found the solution - the issue is that currently we were not trusting __all__ and still excluding items from all that were imported from another location unless :allowed-package-names: was present. I think this doesn't make sense - I'm pretty sure that most people documenting a module that contains __all__ will be expecting all items in __all__ to be included in the API docs, so I've made it so that the onlylocals option is ignored if __all__ is present.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.29%. Comparing base (d671dd5) to head (7673171).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
+ Coverage   90.25%   90.29%   +0.04%     
==========================================
  Files          28       30       +2     
  Lines        1201     1206       +5     
==========================================
+ Hits         1084     1089       +5     
  Misses        117      117              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bsipocz]

Can we have a test where we import from a non-private module? I mean those cases worked (before this PR), so I just want to make sure that keeps being the case.
(although I see cases where they listed in multiple __all__s, probably those should be cleaned up, and (which is beyond scope here) preferably errored out by sphinx-automodapi?)

[astrofrog]

Sounds good, will adjust the test

[nstarman]

I'm not familiar with the internals of this package. I know it uses autosummary and has templates.
From that info, I would expect autosummary to generate the stub files via autodoc

A autosummary directive also generates short “stub” files for the entries listed in its content. These files by default contain only the corresponding sphinx.ext.autodoc directive, but can be customized with templates. (link)

The relevant autodoc directive would then have the :members: option which naturally uses __all__.

Is that what's happening here?

[astrofrog]

@nstarman we actually generate the stub files ourselves and they contain individual eg autoclass calls so the fix has to be and is in our code.

[nstarman]

Then all LGTM. The results files seem reasonable :)

[astrofrog]

In future we might want to re examine using Sphinx directly to generate all the API docs (this package predates the ability of Sphinx to generate stub files)

[bsipocz]

In future we might want to re examine using Sphinx directly to generate all the API docs (this package predates the ability of Sphinx to generate stub files)

I really wondered about this, too as a lot has happened in the past 10 years on sphinx land...

[astrofrog]

@bsipocz - I adjusted the test as you requested to include a class imported from a public submodule

[pllim]

Re: Using Sphinx directly -- We have issue at #147

[bsipocz]

Looks good now, thanks @astrofrog!

[bsipocz]

Re: Using Sphinx directly -- We have issue at #147

Yes, I think we're circling back to that question but I can't recall if someone looked into it carefully and made a full assessment. Btw, things like these can be funded short projects; as it's absolutely in the remit of long-term sustainability and maintenance (and IMO has a better impact on the project than bombarding the codebase with automated ruff and such refactorings forcing a lot of rebase and conflicts on PRs and such)

[pllim]

funded short projects

Would be nice! Cycle 4 already ongoing... I wonder if someone wants to take this on for Cycle 5 or some other alternate funding.

[pllim]

Thanks, all! @astrofrog , do you plan to do a release here or you need help?

[astrofrog]

Already done! (v0.18)