Check autodoc's exclude_members before mock. by jezdez · Pull Request #8653 · sphinx-doc/sphinx

jezdez · 2021-01-04T18:11:25Z

This prevents accidently checking for the sphinx_mock attribute on objects that may be proxies or other lazily evaluated objects that shouldn't be documented and are included in exclude_members.

Subject: Check autodoc's exclude_members before mock.

Feature or Bugfix

Bugfix

Purpose

The purpose is to make sure that laziliy evaluated objects aren't accidently triggering side-effects in this code path, such as Flask objects g, request, current_app etc when they are module globals.

Detail

Changes the order of evaluation when considering the members to document, evaluating exclude_members first.

Relates

Classes that inherit mocked imports do not generate docstrings using autodoc. #8164

This prevents accidently checking for the __sphinx_mock__ attribute on objects that may be proxies or other lazily evaluated objects that shouldn't be documented and are included in exclude_members.

tk0miya · 2021-01-05T14:56:39Z

Thank you for the proposal. But I'm thinking #8656 is better for the case of requests module because this is a workaround.

jezdez · 2021-01-06T12:09:22Z

@tk0miya Ah, hm, I'm not sure I agree since it's not just AttributeError that could be raised when getattr fails, here's the traceback of the error that was relating to having Flask's thread local proxies as module globals:

Traceback (most recent call last):
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/cmd/build.py", line 280, in build_main
    app.build(args.force_all, filenames)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/application.py", line 352, in build
    self.builder.build_update()
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 296, in build_update
    self.build(to_build,
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 310, in build
    updated_docnames = set(self.read())
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 417, in read
    self._read_serial(docnames)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 438, in _read_serial
    self.read_doc(docname)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/builders/__init__.py", line 478, in read_doc
    doctree = read_doc(self.app, self.env, self.env.doc2path(docname))
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/io.py", line 221, in read_doc
    pub.publish()
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/core.py", line 217, in publish
    self.document = self.reader.read(self.source, self.parser,
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/io.py", line 126, in read
    self.parse()
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/readers/__init__.py", line 77, in parse
    self.parser.parse(self.input, document)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/parsers.py", line 104, in parse
    self.statemachine.run(inputlines, document, inliner=self.inliner)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 170, in run
    results = StateMachineWS.run(self, input_lines, input_offset,
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 241, in run
    context, next_state, result = self.check_line(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 459, in check_line
    return method(match, context, next_state)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2769, in underline
    self.section(title, source, style, lineno - 1, messages)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 327, in section
    self.new_subsection(title, lineno, messages)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 393, in new_subsection
    newabsoffset = self.nested_parse(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 281, in nested_parse
    state_machine.run(block, input_offset, memo=self.memo,
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 196, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 241, in run
    context, next_state, result = self.check_line(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 459, in check_line
    return method(match, context, next_state)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2769, in underline
    self.section(title, source, style, lineno - 1, messages)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 327, in section
    self.new_subsection(title, lineno, messages)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 393, in new_subsection
    newabsoffset = self.nested_parse(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 281, in nested_parse
    state_machine.run(block, input_offset, memo=self.memo,
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 196, in run
    results = StateMachineWS.run(self, input_lines, input_offset)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 241, in run
    context, next_state, result = self.check_line(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/statemachine.py", line 459, in check_line
    return method(match, context, next_state)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2342, in explicit_markup
    nodelist, blank_finish = self.explicit_construct(match)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2354, in explicit_construct
    return method(self, expmatch)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2096, in directive
    return self.run_directive(
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/docutils/parsers/rst/states.py", line 2146, in run_directive
    result = directive_instance.run()
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/ext/autodoc/directive.py", line 146, in run
    documenter.generate(more_content=self.content)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/ext/autodoc/__init__.py", line 959, in generate
    self.document_members(all_members)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/ext/autodoc/__init__.py", line 821, in document_members
    for (mname, member, isattr) in self.filter_members(members, want_all):
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/ext/autodoc/__init__.py", line 734, in filter_members
    if ismock(member):
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/sphinx/ext/autodoc/mock.py", line 156, in ismock
    if not hasattr(subject, '__sphinx_mock__'):
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/werkzeug/local.py", line 306, in _get_current_object
    return self.__local()
  File "/Users/jezdez/Code/git/python-dockerflow/.tox/py38-docs/lib/python3.8/site-packages/flask/globals.py", line 52, in _find_app
    raise RuntimeError(_app_ctx_err_msg)
RuntimeError: Working outside of application context.

tk0miya · 2021-01-06T17:06:37Z

My understanding is this helps avoiding the error via :exclude-members: option. It means this requires users to skip the members that raises an error on getattr, manually. I feel it is a workaround. And #8656 can ignore all of errors on getattr. It uses safe_getattr() to handle all kinds of exceptions. So I consider my PR makes this unnecessary, right?

jezdez · 2021-01-06T19:55:58Z

It uses safe_getattr() to handle all kinds of exceptions

Ah, I missed that part, apologies, thanks for elaborating @tk0miya!

jace · 2021-01-07T11:45:08Z

Perhaps safe_getattr needs to be complemented with safe_hasattr? As the hasattr call is breaking on proxies.

tk0miya · 2021-01-07T16:13:43Z

It would be better to implement safe_hasattr to optimize it. But it is not required. Original hasattr() uses getattr() internally. So we can implement hasattr() using getattr().

Check autodoc's exclude_members before mock.

73096bc

This prevents accidently checking for the __sphinx_mock__ attribute on objects that may be proxies or other lazily evaluated objects that shouldn't be documented and are included in exclude_members.

jenshnielsen mentioned this pull request Jan 5, 2021

Build docs with Sphinx 3.4.1 microsoft/Qcodes#2560

Merged

tk0miya added extensions:autodoc type:bug closed:wontfix labels Jan 5, 2021

tk0miya added this to the 3.4.3 milestone Jan 5, 2021

tk0miya closed this Jan 7, 2021

github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Check autodoc's exclude_members before mock.#8653

Check autodoc's exclude_members before mock.#8653
jezdez wants to merge 1 commit intosphinx-doc:3.xfrom
jezdez:patch-1

jezdez commented Jan 4, 2021

Uh oh!

tk0miya commented Jan 5, 2021

Uh oh!

jezdez commented Jan 6, 2021

Uh oh!

tk0miya commented Jan 6, 2021

Uh oh!

jezdez commented Jan 6, 2021

Uh oh!

jace commented Jan 7, 2021

Uh oh!

tk0miya commented Jan 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jezdez commented Jan 4, 2021

Feature or Bugfix

Purpose

Detail

Relates

Uh oh!

tk0miya commented Jan 5, 2021

Uh oh!

jezdez commented Jan 6, 2021

Uh oh!

tk0miya commented Jan 6, 2021

Uh oh!

jezdez commented Jan 6, 2021

Uh oh!

jace commented Jan 7, 2021

Uh oh!

tk0miya commented Jan 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants