Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Correctly detect publicity of modules inside directories #494

Merged
merged 25 commits into from Aug 22, 2020
Merged

Correctly detect publicity of modules inside directories #494

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3b730ac
Fix Module.is_public() when the module is not a the root
tibdex May 4, 2020
be1aafe
Add PR reference
tibdex May 4, 2020
fcca361
Use pathlib
tibdex Jul 6, 2020
716332e
Merge branch 'master' into fix-module_is_public
tibdex Jul 6, 2020
eb201f3
Use pathlib instead of os for test_module_publicity
gbroques Jul 19, 2020
0bac2fc
Update release notes for #493
gbroques Jul 19, 2020
60e8ba0
Resolve merge conflicts in release_notes.rst
gbroques Jul 19, 2020
10e1e7a
Use forward slash '/' operator instead of .joinpath()
gbroques Jul 20, 2020
c911e9f
Fix pull-request number in release notes
gbroques Jul 20, 2020
123025c
Fix publicity of module in private package
gbroques Jul 20, 2020
8c07f7f
Update test_module_publicity docstring
gbroques Jul 21, 2020
84fa77f
Add test for directory starting with double underscore
gbroques Jul 27, 2020
44d661c
Make packages containing double-underscore public
gbroques Aug 1, 2020
a7f7b6c
Add test to assert __init__ module is public
gbroques Aug 1, 2020
ef7f5a1
Make modules in a __private_package private
gbroques Aug 1, 2020
53a246e
Fix lint errors from lines being too long
gbroques Aug 1, 2020
6d62651
Update publicity.rst with more information
gbroques Aug 1, 2020
cec5793
Parameterize module publicity tests and include .py file extension in…
gbroques Aug 1, 2020
4b57b42
Make module publicity determination respect $PYTHONPATH
gbroques Aug 6, 2020
f807365
Fix line-length issue
gbroques Aug 6, 2020
4bb734d
Resolve merge conflicts in release_notes.rst
gbroques Aug 6, 2020
cbf1c60
Reword comment
gbroques Aug 7, 2020
dde693f
Add tests with the same path over different sys.path cases
gbroques Aug 7, 2020
2f6ab9e
Add note about checking sys.path for determining publicity
gbroques Aug 10, 2020
0e5648c
Apply suggestions from code review
samj1912 Aug 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/release_notes.rst
View file
Open in desktop
Expand Up @@ -20,7 +20,7 @@ Bug Fixes
The bug caused some argument names to go unreported in D417 (#448).
* Fixed an issue where skipping errors on module level docstring via #noqa
failed when there where more prior comments (#446).

* Correctly detect publicity of modules inside directories (#470, #494).

5.0.2 - January 8th, 2020
---------------------------
Expand Down
13 changes: 12 additions & 1 deletion src/pydocstyle/parser.py
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@
from itertools import chain, dropwhile
from re import compile as re
from io import StringIO
from pathlib import Path

from .utils import log

Expand Down Expand Up @@ -117,7 +118,17 @@ def is_public(self):

This helps determine if it requires a docstring.
"""
return not self.name.startswith('_') or self.name.startswith('__')
module_name = Path(self.name).name
return (
not self._is_inside_private_package() and
(not module_name.startswith('_') or module_name.startswith('__'))
)

def _is_inside_private_package(self):
"""Return True if the module is inside a private package."""
path = Path(self.name)
package_names = path.parts[:-1]
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a fundamental issue here. The publicity of a module doesn't depend on its entire path, just the path leading to directory inside one of the paths in PYTHONPATH.

So let's say I have a module in path /_foo/bar/baz.py. If PYTHONPATH is set to /, then baz.py is private. If PYTHONPATH is set to /_foo/ then baz.py is public.

Fortunately sys.path is a list of all the filepaths in PYTHONPATH, so this can be relatively simply implemented. Something like:

def _is_inside_private_package(name):
    path = Path(name).parent  # Ignore the actual module's name
    syspath = [Path(p) for p in sys.path]  # Convert to pathlib.Path.

    while path != path.parent and path not in syspath:  # Bail if we are at the root directory or in `PYTHONPATH`.
        if _is_private_name(path.name):
            print(f"Stopping at private package: {path}")
            return True
        path = path.parent

    return False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Let me try and incorporate your suggestion, and ideally add a unit test to capture this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please let me know if 4b57b42 addresses your comment, and if there are any other changes desired.

return any([package.startswith('_') for package in package_names])
samj1912 marked this conversation as resolved.
Show resolved Hide resolved

def __str__(self):
return 'at module level'
Expand Down
28 changes: 23 additions & 5 deletions src/tests/parser_test.py
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@
import sys
import pytest
import textwrap
from pathlib import Path

from pydocstyle.parser import Parser, ParseError

Expand Down Expand Up @@ -562,18 +563,35 @@ def test_matrix_multiplication_with_decorators(code):
assert inner_function.decorators[0].name == 'a'


def test_module_publicity():
"""Test that a module that has a single leading underscore is private."""
@pytest.mark.parametrize("parent_path", (
Path("some") / "directory",
Path("")
))
def test_module_publicity(parent_path):
samj1912 marked this conversation as resolved.
Show resolved Hide resolved
"""Test module publicity.

Modules containing a single leading underscore are private,
and modules within a private package are private.

Private packages contain a single leading underscore.
"""
parser = Parser()
code = CodeSnippet("")

module = parser.parse(code, "filepath")
module = parser.parse(code, str(parent_path / "filepath"))
assert module.is_public

module = parser.parse(code, "_filepath")
module = parser.parse(code, str(parent_path / "_private_pkg" / "filepath"))
assert not module.is_public

module = parser.parse(code, str(
parent_path / "_private_pkg" / "some_pkg" / "filepath"))
assert not module.is_public

module = parser.parse(code, str(parent_path / "_filepath"))
assert not module.is_public

module = parser.parse(code, "__filepath")
module = parser.parse(code, str(parent_path / "__filepath"))
assert module.is_public


Expand Down