Correctly detect publicity of modules inside directories #494
Changes from all commits
3b730ac
be1aafe
fcca361
716332e
eb201f3
0bac2fc
60e8ba0
10e1e7a
c911e9f
123025c
8c07f7f
84fa77f
44d661c
a7f7b6c
ef7f5a1
53a246e
6d62651
cec5793
4b57b42
f807365
4bb734d
cbf1c60
dde693f
2f6ab9e
0e5648c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,3 +54,6 @@ docs/snippets/error_code_table.rst | |
|
||
# PyCharm files | ||
.idea | ||
|
||
# VS Code | ||
.vscode/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
"""Python code parser.""" | ||
|
||
import sys | ||
import textwrap | ||
import tokenize as tk | ||
from itertools import chain, dropwhile | ||
from re import compile as re | ||
from io import StringIO | ||
from pathlib import Path | ||
|
||
from .utils import log | ||
|
||
|
@@ -117,7 +119,36 @@ 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).stem | ||
return ( | ||
not self._is_inside_private_package() and | ||
self._is_public_name(module_name) | ||
) | ||
|
||
def _is_inside_private_package(self): | ||
"""Return True if the module is inside a private package.""" | ||
path = Path(self.name).parent # Ignore the actual module's name. | ||
syspath = [Path(p) for p in sys.path] # Convert to pathlib.Path. | ||
|
||
# Bail if we are at the root directory or in `PYTHONPATH`. | ||
while path != path.parent and path not in syspath: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we should iterate all the way to the root. But only till the current directory? Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. edit - The current working directory will be in the sys path in certain conditions https://docs.python.org/3.8/library/sys.html#sys.path There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did some tests with the
Maybe your right? Please advise. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am just wondering about this use case because if we iterate all they way till root, there might be folders with an underscore in their name. On the other hand this means that pydocstyle will have different outputs on the same input based on the In the future if we decide to change this behavior we can always put in a PR - @Nurdok what do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 for keeping as is and documenting it. A user can always get the current-directory-behavior by calling |
||
if self._is_private_name(path.name): | ||
return True | ||
path = path.parent | ||
|
||
return False | ||
|
||
def _is_public_name(self, module_name): | ||
"""Determine whether a "module name" (i.e. module or package name) is public.""" | ||
return ( | ||
not module_name.startswith('_') or ( | ||
module_name.startswith('__') and module_name.endswith('__') | ||
) | ||
) | ||
|
||
def _is_private_name(self, module_name): | ||
"""Determine whether a "module name" (i.e. module or package name) is private.""" | ||
return not self._is_public_name(module_name) | ||
|
||
def __str__(self): | ||
return 'at module level' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
import sys | ||
import pytest | ||
import textwrap | ||
from pathlib import Path | ||
|
||
from pydocstyle.parser import Parser, ParseError | ||
|
||
|
@@ -562,19 +563,75 @@ 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("public_path", ( | ||
Path(""), | ||
Path("module.py"), | ||
Path("package") / "module.py", | ||
Path("package") / "__init__.py", | ||
Path("") / "package" / "module.py", | ||
Path("") / "__dunder__" / "package" / "module.py" | ||
)) | ||
def test_module_publicity_with_public_path(public_path): | ||
"""Test module publicity with public path. | ||
|
||
Module names such as my_module.py are considered public. | ||
|
||
Special "dunder" modules, | ||
with leading and trailing double-underscores (e.g. __init__.py) are public. | ||
|
||
The same rules for publicity apply to both packages and modules. | ||
""" | ||
parser = Parser() | ||
code = CodeSnippet("") | ||
|
||
module = parser.parse(code, "filepath") | ||
module = parser.parse(code, str(public_path)) | ||
assert module.is_public | ||
|
||
module = parser.parse(code, "_filepath") | ||
|
||
@pytest.mark.parametrize("private_path", ( | ||
# single underscore | ||
Path("_private_module.py"), | ||
Path("_private_package") / "module.py", | ||
Path("_private_package") / "package" / "module.py", | ||
Path("") / "_private_package" / "package" / "module.py", | ||
|
||
# double underscore | ||
Path("__private_module.py"), | ||
Path("__private_package") / "module.py", | ||
Path("__private_package") / "package" / "module.py", | ||
Path("") / "__private_package" / "package" / "module.py" | ||
)) | ||
def test_module_publicity_with_private_paths(private_path): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also add some tests with the same path over different There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate on how to do this please? EDIT: It looks like Is something like this desirable (updated in dde693f)? @pytest.mark.parametrize("syspath,is_public", (
("/", False),
("_foo/", True),
))
def test_module_publicity_with_different_sys_path(syspath, is_public, monkeypatch):
"""Test module publicity for same path and different sys.path."""
parser = Parser()
code = CodeSnippet("")
monkeypatch.syspath_prepend(syspath)
path = Path("_foo") / "bar" / "baz.py"
module = parser.parse(code, str(path))
assert module.is_public == is_public |
||
"""Test module publicity with private path. | ||
|
||
Module names starting with single or double-underscore are private. | ||
For example, _my_private_module.py and __my_private_module.py. | ||
|
||
Any module within a private package is considered private. | ||
|
||
The same rules for publicity apply to both packages and modules. | ||
""" | ||
parser = Parser() | ||
code = CodeSnippet("") | ||
module = parser.parse(code, str(private_path)) | ||
assert not module.is_public | ||
|
||
module = parser.parse(code, "__filepath") | ||
assert module.is_public | ||
|
||
@pytest.mark.parametrize("syspath,is_public", ( | ||
("/", False), | ||
("_foo/", True), | ||
)) | ||
def test_module_publicity_with_different_sys_path(syspath, | ||
is_public, | ||
monkeypatch): | ||
"""Test module publicity for same path and different sys.path.""" | ||
parser = Parser() | ||
code = CodeSnippet("") | ||
|
||
monkeypatch.syspath_prepend(syspath) | ||
|
||
path = Path("_foo") / "bar" / "baz.py" | ||
module = parser.parse(code, str(path)) | ||
assert module.is_public == is_public | ||
|
||
|
||
def test_complex_module(): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also update the docs to include our new behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which new behavior are you referring to?
Do you have suggestions for wording it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - I meant the behavior around syspath. That may catch some people off guard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can mention it as a "Note" somewhere in the "How publicity is determined" section.
Thoughts?
EDIT: I suppose this is dependent upon your other comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep this comment to what you suggested for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in commit 2f6ab9e.