Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with :skip: introduced by :include: feature #142

Merged
merged 7 commits into from
Dec 29, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Changes in sphinx-automodapi
0.15.0 (unreleased)
-------------------

- Fixed issue with ``:skip:`` introduced by ``:include:`` feature. [#142]

0.14.0 (2021-12-22)
-------------------

Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ filterwarnings =
ignore:The `docutils\.parsers\.rst\.directive\.html` module will be removed:DeprecationWarning
ignore:'contextfunction' is renamed to 'pass_context':DeprecationWarning
ignore:'environmentfilter' is renamed to 'pass_environment':DeprecationWarning
ignore:distutils Version classes are deprecated:DeprecationWarning

[flake8]
max-line-length = 125
Expand Down
6 changes: 3 additions & 3 deletions sphinx_automodapi/automodapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -411,12 +411,12 @@ def _mod_info(modname, toskip=[], include=[], onlylocals=True):

hascls = hasfunc = hasother = False

skips = []
skips = toskip.copy()
for localnm, fqnm, obj in zip(*find_mod_objs(modname, onlylocals=onlylocals)):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new :include: feature has a similar problem as was introduced for :skip:. Here's a test that fails on this branch:

am_replacer_include_stdlib_str = """
This comes before

.. automodapi:: sphinx_automodapi.tests.example_module.stdlib
    :include: add

This comes after
"""

am_replacer_include_stdlib_expected = """
This comes before


sphinx_automodapi.tests.example_module.stdlib Module
----------------------------------------------------

.. automodule:: sphinx_automodapi.tests.example_module.stdlib

Functions
^^^^^^^^^

.. automodsumm:: sphinx_automodapi.tests.example_module.stdlib
    :functions-only:
    :toctree: api
    :skip: Path,time


This comes after
""".format(empty='')


def test_am_replacer_include_stdlib(tmpdir):
    """
    Tests using the ":include: option in an ".. automodapi::"
    in the presence of objects imported from the standard library.
    """

    with open(tmpdir.join('index.rst').strpath, 'w') as f:
        f.write(am_replacer_include_stdlib_str.format(options=''))

    run_sphinx_in_tmpdir(tmpdir)

    with open(tmpdir.join('index.rst.automodapi').strpath) as f:
        result = f.read()

    assert result == am_replacer_include_stdlib_expected
    ```

Copy link
Member Author

@adrn adrn Dec 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is an issue -- the function names would only need to be skipped if you added the stdlib module names to :allowed-package-names:. For example,

am_replacer_include_stdlib_str = """
This comes before

.. automodapi:: sphinx_automodapi.tests.example_module.stdlib
    :include: add
    :allowed-package-names: pathlib, datetime, sphinx_automodapi

This comes after
"""

am_replacer_include_stdlib_expected = """
This comes before


sphinx_automodapi.tests.example_module.stdlib Module
----------------------------------------------------

.. automodule:: sphinx_automodapi.tests.example_module.stdlib

Functions
^^^^^^^^^

.. automodsumm:: sphinx_automodapi.tests.example_module.stdlib
    :functions-only:
    :toctree: api
    :skip: Path,time
    :allowed-package-names: pathlib,datetime,sphinx_automodapi


This comes after
""".format(empty='')

Then the test passes. Otherwise, find_mod_objs() has onlylocals=True and ignores the imported names.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay! That makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you at least want to put this test in CI before merge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it's a worthy test, given what @adrn told me -- it makes sense that :skip: doesn't appear, but I'm not sure that we need to assert that it is not present.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added one more check that doing what I said in the comment above makes your test pass -- I think that is still worth including!

if localnm in toskip or (include and localnm not in include):
if include and localnm not in include and localnm not in skips:
skips.append(localnm)

else:
elif localnm not in toskip:
hascls = hascls or inspect.isclass(obj)
hasfunc = hasfunc or inspect.isroutine(obj)
hasother = hasother or (not inspect.isclass(obj) and
Expand Down
15 changes: 15 additions & 0 deletions sphinx_automodapi/tests/example_module/stdlib.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
"""
A module that imports objects from the standard library.
"""
from pathlib import Path
from datetime import time


__all__ = ['Path', 'time', 'add']


def add(a, b):
"""
Add two numbers
"""
return a + b
51 changes: 51 additions & 0 deletions sphinx_automodapi/tests/test_automodapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,57 @@ def test_am_replacer_skip(tmpdir):
assert result == am_replacer_skip_expected


am_replacer_skip_stdlib_str = """
This comes before

.. automodapi:: sphinx_automodapi.tests.example_module.stdlib
:skip: time
:skip: Path

This comes after
"""


am_replacer_skip_stdlib_expected = """
This comes before


sphinx_automodapi.tests.example_module.stdlib Module
----------------------------------------------------

.. automodule:: sphinx_automodapi.tests.example_module.stdlib

Functions
^^^^^^^^^

.. automodsumm:: sphinx_automodapi.tests.example_module.stdlib
:functions-only:
:toctree: api
:skip: time,Path


This comes after
""".format(empty='')


def test_am_replacer_skip_stdlib(tmpdir):
"""
Tests using the ":skip:" option in an ".. automodapi::"
that skips objects imported from the standard library.
This is a regression test for #141
"""

with open(tmpdir.join('index.rst').strpath, 'w') as f:
f.write(am_replacer_skip_stdlib_str.format(options=''))

run_sphinx_in_tmpdir(tmpdir)

with open(tmpdir.join('index.rst.automodapi').strpath) as f:
result = f.read()

assert result == am_replacer_skip_stdlib_expected


am_replacer_include_str = """
This comes before

Expand Down