Fix #5104: apidoc: Interface of sphinx.apidoc:main() has changed

[tk0miya]

Feature or Bugfix

• Bugfix

Purpose

• refs: sphinx-build: error: argument -d/--maxdepth: invalid int value #5104

[adamjstewart]

Doesn't this suffer from the same problem that was fixed in #3668? Shouldn't it be argv=sys.argv[1:] and _main(argv)?

[tk0miya]

sphinx.apidoc:main() is only provided to keep compatibility with 1.6 or older. So it should not omit the first item of sys.argv. On the other hand, sphinx.ext.apidoc:main() takes sys.argv[1:].
I think this is correct.

[adamjstewart]

@tk0miya Shouldn't both sphinx.ext.apidoc.main() and sphinx.apidoc.main() behave identically? Currently, both default to sys.argv[1:], the only difference is that sphinx.apidoc.main() throws away the first argument it is given, while sphinx.ext.apidoc.main() does not. This seems like it is reverting the behavior introduced in #3668.

[tk0miya]

Yes, #3668 is incompatible change to past release. So this revert the change. The most important feature of sphinx.apidoc:main() is taking full sys.argv as Sphinx-1.6 doing.

[adamjstewart]

Why revert #3668? It fixed an important bug where the first argument passed to sphinx.apidoc.main is completely ignored with no warning. Reintroducing bugs to preserver compatibility with old releases doesn't make sense to me.

[tk0miya]

sphinx.apidoc:main() is only used to keep compatibility with past release. It should be same as 1.6's one doing. That means it should take whole of sys.argv, not sys.argv[1:].

The change of #3668 is merged into sphinx.ext.apidoc:main(). It is successor of sphinx.apidoc module. I think its interface is same as what you want.

It fixed an important bug where the first argument passed to sphinx.apidoc.main is completely ignored with no warning.

I don't think this is not a bug. The main function is not public API. So there are no specification.

[adamjstewart]

Ah, I see. I guess I assumed that #3668 had been merged and released before 1.7.

I don't think this is not a bug. The main function is not public API. So there are no specification.

Fair enough. Is sphinx.ext.apidoc.main() a public API?

[tk0miya]

Fair enough. Is sphinx.ext.apidoc.main() a public API?

It's not. The public APIs are listed in document:
http://www.sphinx-doc.org/en/master/extdev/index.html

But it does not mean other functions are fragile. We'll work to keep all of functions stable. So please let me know if something goes wrong.

[adamjstewart]

If sphinx.ext.apidoc.main isn't public, is there a recommended way to automatically generate API docs? Is it preferable to call sphinx-apidoc in a subprocess?

[tk0miya]

AFAIK, apidoc is useful to create skelton of API document project. So it is not needed to invoke it through APIs. For that purpose, we provides sphinx.ext.autosummary module. How about this?
Please let me know your goal.

Anyway, your request is not related with this bug. So I'd like to merge this if this works fine. Could you move to new issue please?

[stephenfin]

If sphinx.ext.apidoc.main isn't public, is there a recommended way to automatically generate API docs? Is it preferable to call sphinx-apidoc in a subprocess?

@adamjstewart try sphinx-contrib/apidoc. We're using this extensively in OpenStack until I can talk @tk0miya around into accepting #4101 😉

AFAIK, apidoc is useful to create skelton of API document project. So it is not needed to invoke it through APIs. For that purpose, we provides sphinx.ext.autosummary module. How about this?
Please let me know your goal.

@tk0miya I'm afraid it's more complicated than that. There are big differences between sphinx.ext.autosummary and sphinx-apidoc. I described these in #4101 (comment). Until we resolve those differences, sphinx-apidoc remains the only option for OpenStack at least.

[stephenfin]

Why revert #3668? It fixed an important bug where the first argument passed to sphinx.apidoc.main is completely ignored with no warning. Reintroducing bugs to preserver compatibility with old releases doesn't make sense to me.

FWIW, #3668 was necessary due to a change in how argparse and optparse work. That defaults to using sys.argv[1:] unless you tell it otherwise, which we needed to do so we could call it programatically for unit tests and the likes. As you've noted though, neither original nor updated version are correct in that there's a disconnect between what we expected in the past (a list of arguments referenced by argv) and what we expect today (an option list of args and kwargs, referenced via arg and kwarg respectively). As a result, this fix seems correct.

[stephenfin]

Looks good to me.

[jakirkham]

...try sphinx-contrib/apidoc.

Thanks for sharing @stephenfin. Glad to hear there are proper tools out there. Have been resorting to hacks like calling sphinx.ext.apidoc.main from conf.py for some time.

If it helps, @tk0miya, lots of people try to do something similar. ( readthedocs/readthedocs.org#1139 ) Am sure they would also appreciate proper support in Sphinx for this use case. :)

[jakirkham]

In any event, would still be great to have this PR land.

[codecov]

Codecov Report

Merging #5106 into 1.7 will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##              1.7    #5106      +/-   ##
==========================================
+ Coverage   82.04%   82.05%   +<.01%     
==========================================
  Files         283      288       +5     
  Lines       37825    38144     +319     
  Branches     5860     5914      +54     
==========================================
+ Hits        31035    31298     +263     
- Misses       5481     5528      +47     
- Partials     1309     1318       +9

Impacted Files	Coverage Δ
sphinx/apidoc.py	0% <0%> (ø)
sphinx/errors.py	68.42% <0%> (ø)
sphinx/search/__init__.py	95.11% <0%> (ø)
sphinx/quickstart.py	0% <0%> (ø)
sphinx/__init__.py	57.5% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e27f6c...f54d886. Read the comment docs.

[tk0miya]

@stephenfin Thank you for your help. it was very helpful to me (because I'm very poor at English...)

[tk0miya]

Merged. Thank you for your comment. Let's talk about the API or modules in another issue.

I'll take a look #4101 later. Please wait a moment.