From 3b730ac0685141ff5ed0ad425b01c9ab64bb1bf8 Mon Sep 17 00:00:00 2001 From: Thibault Derousseaux Date: Mon, 4 May 2020 14:44:20 +0200 Subject: [PATCH 01/22] Fix Module.is_public() when the module is not a the root --- docs/release_notes.rst | 1 + src/pydocstyle/parser.py | 4 +++- src/tests/parser_test.py | 13 +++++++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index 13c677fb..ae405bb4 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -15,6 +15,7 @@ Bug Fixes * Update convention support documentation (#386, #393) * Detect inner asynchronous functions for D202 (#467) +* Correctly detect publicity of modules inside directories 5.0.2 - January 8th, 2020 --------------------------- diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 76b3dab7..176db174 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -1,5 +1,6 @@ """Python code parser.""" +import os import textwrap import tokenize as tk from itertools import chain, dropwhile @@ -117,7 +118,8 @@ def is_public(self): This helps determine if it requires a docstring. """ - return not self.name.startswith('_') or self.name.startswith('__') + module_name = os.path.basename(self.name) + return not module_name.startswith('_') or module_name.startswith('__') def __str__(self): return 'at module level' diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 832d1b52..0f7af54b 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -1,6 +1,7 @@ """Parser tests.""" import io +import os import sys import pytest import textwrap @@ -562,18 +563,22 @@ def test_matrix_multiplication_with_decorators(code): assert inner_function.decorators[0].name == 'a' -def test_module_publicity(): +@pytest.mark.parametrize("parent_path", ( + os.path.join("some", "directory"), + "" +)) +def test_module_publicity(parent_path): """Test that a module that has a single leading underscore is private.""" parser = Parser() code = CodeSnippet("") - module = parser.parse(code, "filepath") + module = parser.parse(code, os.path.join(parent_path, "filepath")) assert module.is_public - module = parser.parse(code, "_filepath") + module = parser.parse(code, os.path.join(parent_path, "_filepath")) assert not module.is_public - module = parser.parse(code, "__filepath") + module = parser.parse(code, os.path.join(parent_path, "__filepath")) assert module.is_public From be1aafefa2112255495d9cc6ac41179a86872b1b Mon Sep 17 00:00:00 2001 From: Thibault Derousseaux Date: Mon, 4 May 2020 14:49:32 +0200 Subject: [PATCH 02/22] Add PR reference --- docs/release_notes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index ae405bb4..01f39949 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -15,7 +15,7 @@ Bug Fixes * Update convention support documentation (#386, #393) * Detect inner asynchronous functions for D202 (#467) -* Correctly detect publicity of modules inside directories +* Correctly detect publicity of modules inside directories (#470) 5.0.2 - January 8th, 2020 --------------------------- From fcca36148f9a912b073fbce7679fc8de743bd77e Mon Sep 17 00:00:00 2001 From: Thibault Derousseaux Date: Mon, 6 Jul 2020 10:08:58 +0200 Subject: [PATCH 03/22] Use pathlib --- src/pydocstyle/parser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 176db174..8ee587b9 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -1,11 +1,11 @@ """Python code parser.""" -import os 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 @@ -118,7 +118,7 @@ def is_public(self): This helps determine if it requires a docstring. """ - module_name = os.path.basename(self.name) + module_name = Path(self.name).name return not module_name.startswith('_') or module_name.startswith('__') def __str__(self): From eb201f34b680dfa3617b86fd28492087b9041ddf Mon Sep 17 00:00:00 2001 From: G Roques Date: Sun, 19 Jul 2020 18:12:14 -0500 Subject: [PATCH 04/22] Use pathlib instead of os for test_module_publicity --- src/tests/parser_test.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 0f7af54b..24cd5e48 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -1,10 +1,10 @@ """Parser tests.""" import io -import os import sys import pytest import textwrap +from pathlib import Path from pydocstyle.parser import Parser, ParseError @@ -564,21 +564,21 @@ def test_matrix_multiplication_with_decorators(code): @pytest.mark.parametrize("parent_path", ( - os.path.join("some", "directory"), - "" + Path("some").joinpath("directory"), + Path("") )) def test_module_publicity(parent_path): """Test that a module that has a single leading underscore is private.""" parser = Parser() code = CodeSnippet("") - module = parser.parse(code, os.path.join(parent_path, "filepath")) + module = parser.parse(code, str(parent_path.joinpath("filepath"))) assert module.is_public - module = parser.parse(code, os.path.join(parent_path, "_filepath")) + module = parser.parse(code, str(parent_path.joinpath("_filepath"))) assert not module.is_public - module = parser.parse(code, os.path.join(parent_path, "__filepath")) + module = parser.parse(code, str(parent_path.joinpath("__filepath"))) assert module.is_public From 0bac2fc36dbbf1a63df48d9c6d123955410407d6 Mon Sep 17 00:00:00 2001 From: G Roques Date: Sun, 19 Jul 2020 18:14:16 -0500 Subject: [PATCH 05/22] Update release notes for #493 --- docs/release_notes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index f4e7dd45..897c3d24 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -17,7 +17,7 @@ Bug Fixes * Detect inner asynchronous functions for D202 (#467) * Fix a bug in parsing Google-style argument description. The bug caused some argument names to go unreported in D417 (#448). -* Correctly detect publicity of modules inside directories (#470). +* Correctly detect publicity of modules inside directories (#470, #493). 5.0.2 - January 8th, 2020 --------------------------- From 10e1e7aad14628d52e14788aa5a5807ee9928631 Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 20 Jul 2020 10:30:51 -0500 Subject: [PATCH 06/22] Use forward slash '/' operator instead of .joinpath() --- src/tests/parser_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 24cd5e48..dd4b75cf 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -564,7 +564,7 @@ def test_matrix_multiplication_with_decorators(code): @pytest.mark.parametrize("parent_path", ( - Path("some").joinpath("directory"), + Path("some") / "directory", Path("") )) def test_module_publicity(parent_path): @@ -572,13 +572,13 @@ def test_module_publicity(parent_path): parser = Parser() code = CodeSnippet("") - module = parser.parse(code, str(parent_path.joinpath("filepath"))) + module = parser.parse(code, str(parent_path / "filepath")) assert module.is_public - module = parser.parse(code, str(parent_path.joinpath("_filepath"))) + module = parser.parse(code, str(parent_path / "_filepath")) assert not module.is_public - module = parser.parse(code, str(parent_path.joinpath("__filepath"))) + module = parser.parse(code, str(parent_path / "__filepath")) assert module.is_public From c911e9f2a2c4f6ecc0087450cea3a3c9c6ae782f Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 20 Jul 2020 17:20:38 -0500 Subject: [PATCH 07/22] Fix pull-request number in release notes --- docs/release_notes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release_notes.rst b/docs/release_notes.rst index a38bc8a1..9e9c4f3f 100644 --- a/docs/release_notes.rst +++ b/docs/release_notes.rst @@ -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, #493). +* Correctly detect publicity of modules inside directories (#470, #494). 5.0.2 - January 8th, 2020 --------------------------- From 123025c7612a6a1e2616ac619323da4c2d7a4108 Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 20 Jul 2020 18:20:38 -0500 Subject: [PATCH 08/22] Fix publicity of module in private package --- src/pydocstyle/parser.py | 11 ++++++++++- src/tests/parser_test.py | 7 +++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index a5470484..0b97a14d 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -119,7 +119,16 @@ def is_public(self): This helps determine if it requires a docstring. """ module_name = Path(self.name).name - return not module_name.startswith('_') or module_name.startswith('__') + 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] + return any([package.startswith('_') for package in package_names]) def __str__(self): return 'at module level' diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index dd4b75cf..0de5a252 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -575,6 +575,13 @@ def test_module_publicity(parent_path): module = parser.parse(code, str(parent_path / "filepath")) assert module.is_public + 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 From 8c07f7fb329c2bf65385dd7ca87a7a7e057e40f1 Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 20 Jul 2020 19:07:17 -0500 Subject: [PATCH 09/22] Update test_module_publicity docstring --- src/tests/parser_test.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 0de5a252..d723c49e 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -568,7 +568,13 @@ def test_matrix_multiplication_with_decorators(code): Path("") )) def test_module_publicity(parent_path): - """Test that a module that has a single leading underscore is private.""" + """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("") From 84fa77f07ecd2d7373a793d1130ec61a75a14bac Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 27 Jul 2020 18:49:17 -0500 Subject: [PATCH 10/22] Add test for directory starting with double underscore --- src/tests/parser_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index d723c49e..20dda276 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -588,6 +588,10 @@ def test_module_publicity(parent_path): parent_path / "_private_pkg" / "some_pkg" / "filepath")) assert not module.is_public + module = parser.parse(code, str( + parent_path / "__dunder_pkg" / "some_pkg" / "filepath")) + assert not module.is_public + module = parser.parse(code, str(parent_path / "_filepath")) assert not module.is_public From 44d661cad04d7d3f9ad7ccab63a31fe8d05cfd71 Mon Sep 17 00:00:00 2001 From: G Roques Date: Fri, 31 Jul 2020 19:51:58 -0500 Subject: [PATCH 11/22] Make packages containing double-underscore public --- src/pydocstyle/parser.py | 12 ++++++++++-- src/tests/parser_test.py | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 0b97a14d..a8ae20da 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -121,14 +121,22 @@ def is_public(self): module_name = Path(self.name).name return ( not self._is_inside_private_package() and - (not module_name.startswith('_') or module_name.startswith('__')) + 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) package_names = path.parts[:-1] - return any([package.startswith('_') for package in package_names]) + return any([self._is_private_name(package) for package in package_names]) + + 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('__') + + 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' diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 20dda276..785bafc0 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -589,8 +589,12 @@ def test_module_publicity(parent_path): assert not module.is_public module = parser.parse(code, str( - parent_path / "__dunder_pkg" / "some_pkg" / "filepath")) - assert not module.is_public + parent_path / "__dunder_pkg__" / "some_pkg" / "filepath")) + assert module.is_public + + module = parser.parse(code, str( + parent_path / "__leading_dunder_pkg" / "some_pkg" / "filepath")) + assert module.is_public module = parser.parse(code, str(parent_path / "_filepath")) assert not module.is_public From a7f7b6cd945a905b856a5098a18e8409dfcb9a1a Mon Sep 17 00:00:00 2001 From: G Roques Date: Fri, 31 Jul 2020 19:55:36 -0500 Subject: [PATCH 12/22] Add test to assert __init__ module is public --- src/tests/parser_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 785bafc0..2aeeeb95 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -592,6 +592,10 @@ def test_module_publicity(parent_path): parent_path / "__dunder_pkg__" / "some_pkg" / "filepath")) assert module.is_public + module = parser.parse(code, str( + parent_path / "some_pkg" / "__init__")) + assert module.is_public + module = parser.parse(code, str( parent_path / "__leading_dunder_pkg" / "some_pkg" / "filepath")) assert module.is_public From ef7f5a1945e2a20f42e5913796892abc478656f9 Mon Sep 17 00:00:00 2001 From: G Roques Date: Sat, 1 Aug 2020 08:50:43 -0500 Subject: [PATCH 13/22] Make modules in a __private_package private --- src/pydocstyle/parser.py | 8 ++++-- src/tests/parser_test.py | 57 ++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index a8ae20da..0630267e 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -118,7 +118,7 @@ def is_public(self): This helps determine if it requires a docstring. """ - module_name = Path(self.name).name + module_name = Path(self.name).stem return ( not self._is_inside_private_package() and self._is_public_name(module_name) @@ -132,7 +132,11 @@ def _is_inside_private_package(self): 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('__') + 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.""" diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 2aeeeb95..1e40d2a5 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -564,47 +564,66 @@ def test_matrix_multiplication_with_decorators(code): @pytest.mark.parametrize("parent_path", ( - Path("some") / "directory", + Path("package") / "another_package", Path("") )) def test_module_publicity(parent_path): """Test module publicity. - Modules containing a single leading underscore are private, - and modules within a private package are private. + Module names such as my_module.py are considered public. - Private packages contain a single leading underscore. + Module names starting with single or double-underscore are private. + For example, _my_private_module.py and __my_private_module.py. + + While special "dunder" modules (i.e. leading and trailing double-underscores), + such as __init__.py, are public. + + The same rules for module name publicity applies to package name publicity. + + Any module within a private package is considered private. """ parser = Parser() code = CodeSnippet("") - module = parser.parse(code, str(parent_path / "filepath")) + # Public assertions + # =========================================================================== + module = parser.parse(code, str(parent_path / "module")) assert module.is_public - 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 / "__dunder_pkg__" / "some_pkg" / "filepath")) + parent_path / "package" / "__init__")) assert module.is_public module = parser.parse(code, str( - parent_path / "some_pkg" / "__init__")) + parent_path / "__dunder__" / "package" / "module")) assert module.is_public + # Private assertions + # =========================================================================== + + # Single Underscore + # ----------------- + module = parser.parse(code, str(parent_path / "_private_module")) + assert not module.is_public + + module = parser.parse(code, str(parent_path / "_private_package" / "module")) + assert not module.is_public + module = parser.parse(code, str( - parent_path / "__leading_dunder_pkg" / "some_pkg" / "filepath")) - assert module.is_public + parent_path / "_private_package" / "package" / "module")) + assert not module.is_public - module = parser.parse(code, str(parent_path / "_filepath")) + # Double Underscore + # ----------------- + module = parser.parse(code, str(parent_path / "__private_module")) assert not module.is_public - module = parser.parse(code, str(parent_path / "__filepath")) - assert module.is_public + module = parser.parse(code, str(parent_path / "__private_package" / "module")) + assert not module.is_public + + module = parser.parse(code, str( + parent_path / "__private_package" / "package" / "module")) + assert not module.is_public def test_complex_module(): From 53a246e74f593e6843520e7a1b916cc53ced05dc Mon Sep 17 00:00:00 2001 From: G Roques Date: Sat, 1 Aug 2020 09:03:01 -0500 Subject: [PATCH 14/22] Fix lint errors from lines being too long --- .gitignore | 3 +++ src/tests/parser_test.py | 10 ++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 36c5d084..6ff33f2c 100644 --- a/.gitignore +++ b/.gitignore @@ -54,3 +54,6 @@ docs/snippets/error_code_table.rst # PyCharm files .idea + +# VS Code +.vscode/ diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index 1e40d2a5..d5215e3b 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -575,8 +575,8 @@ def test_module_publicity(parent_path): Module names starting with single or double-underscore are private. For example, _my_private_module.py and __my_private_module.py. - While special "dunder" modules (i.e. leading and trailing double-underscores), - such as __init__.py, are public. + While special "dunder" modules, + with leading and trailing double-underscores (e.g. __init__.py) are public. The same rules for module name publicity applies to package name publicity. @@ -606,7 +606,8 @@ def test_module_publicity(parent_path): module = parser.parse(code, str(parent_path / "_private_module")) assert not module.is_public - module = parser.parse(code, str(parent_path / "_private_package" / "module")) + module = parser.parse(code, str( + parent_path / "_private_package" / "module")) assert not module.is_public module = parser.parse(code, str( @@ -618,7 +619,8 @@ def test_module_publicity(parent_path): module = parser.parse(code, str(parent_path / "__private_module")) assert not module.is_public - module = parser.parse(code, str(parent_path / "__private_package" / "module")) + module = parser.parse(code, str( + parent_path / "__private_package" / "module")) assert not module.is_public module = parser.parse(code, str( From 6d6265188ce5c2a573af4ade7dd040ac454d2402 Mon Sep 17 00:00:00 2001 From: G Roques Date: Sat, 1 Aug 2020 09:32:07 -0500 Subject: [PATCH 15/22] Update publicity.rst with more information --- docs/snippets/publicity.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/snippets/publicity.rst b/docs/snippets/publicity.rst index e36d3194..dd165a11 100644 --- a/docs/snippets/publicity.rst +++ b/docs/snippets/publicity.rst @@ -10,7 +10,9 @@ Publicity for all constructs is determined as follows: a construct is considered *public* if - 1. Its immediate parent is public *and* -2. Its name does not contain a single leading underscore. +2. Its name does *not* start with a single or double underscore. + + a. Note, names that start and end with a double underscore are *public* (e.g. ``__init__.py``). A construct's immediate parent is the construct that contains it. For example, a method's parent is a class object. A class' parent is usually a module, but From cec57934bf781f3d4956bdcc2c8c62bd6123cc73 Mon Sep 17 00:00:00 2001 From: G Roques Date: Sat, 1 Aug 2020 15:58:33 -0500 Subject: [PATCH 16/22] Parameterize module publicity tests and include .py file extension in test path parameters --- src/tests/parser_test.py | 79 +++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 46 deletions(-) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index d5215e3b..fc084e69 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -563,68 +563,55 @@ def test_matrix_multiplication_with_decorators(code): assert inner_function.decorators[0].name == 'a' -@pytest.mark.parametrize("parent_path", ( - Path("package") / "another_package", - Path("") +@pytest.mark.parametrize("public_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(parent_path): - """Test module publicity. +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. - Module names starting with single or double-underscore are private. - For example, _my_private_module.py and __my_private_module.py. - - While special "dunder" modules, + Special "dunder" modules, with leading and trailing double-underscores (e.g. __init__.py) are public. - The same rules for module name publicity applies to package name publicity. - - Any module within a private package is considered private. + The same rules for publicity apply to both packages and modules. """ parser = Parser() code = CodeSnippet("") - - # Public assertions - # =========================================================================== - module = parser.parse(code, str(parent_path / "module")) + module = parser.parse(code, str(public_path)) assert module.is_public - module = parser.parse(code, str( - parent_path / "package" / "__init__")) - assert module.is_public - - module = parser.parse(code, str( - parent_path / "__dunder__" / "package" / "module")) - assert module.is_public - # Private assertions - # =========================================================================== +@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", - # Single Underscore - # ----------------- - module = parser.parse(code, str(parent_path / "_private_module")) - assert not module.is_public - - module = parser.parse(code, str( - parent_path / "_private_package" / "module")) - assert not module.is_public - - module = parser.parse(code, str( - parent_path / "_private_package" / "package" / "module")) - assert not module.is_public + # 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): + """Test module publicity with private path. - # Double Underscore - # ----------------- - module = parser.parse(code, str(parent_path / "__private_module")) - assert not module.is_public + Module names starting with single or double-underscore are private. + For example, _my_private_module.py and __my_private_module.py. - module = parser.parse(code, str( - parent_path / "__private_package" / "module")) - assert not module.is_public + Any module within a private package is considered private. - module = parser.parse(code, str( - parent_path / "__private_package" / "package" / "module")) + 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 From 4b57b42da6007dba708ad0052189a9e0bebda426 Mon Sep 17 00:00:00 2001 From: G Roques Date: Wed, 5 Aug 2020 19:11:01 -0500 Subject: [PATCH 17/22] Make module publicity determination respect $PYTHONPATH --- src/pydocstyle/parser.py | 13 ++++++++++--- src/tests/parser_test.py | 1 + 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 0630267e..b3e4bfac 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -1,5 +1,6 @@ """Python code parser.""" +import sys import textwrap import tokenize as tk from itertools import chain, dropwhile @@ -126,9 +127,15 @@ def is_public(self): 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] - return any([self._is_private_name(package) for package in package_names]) + path = Path(self.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 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.""" diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index fc084e69..ced3b653 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -564,6 +564,7 @@ def test_matrix_multiplication_with_decorators(code): @pytest.mark.parametrize("public_path", ( + Path(""), Path("module.py"), Path("package") / "module.py", Path("package") / "__init__.py", From f8073657a6b0a63c89d33b2c2a7b2a9ffe25e306 Mon Sep 17 00:00:00 2001 From: G Roques Date: Thu, 6 Aug 2020 18:35:25 -0500 Subject: [PATCH 18/22] Fix line-length issue --- src/pydocstyle/parser.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index b3e4bfac..a1ee674d 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -130,7 +130,8 @@ def _is_inside_private_package(self): path = Path(self.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`. + # 2nd condition is to bail if we are at the root directory or in `PYTHONPATH`. + while path != path.parent and path not in syspath: if self._is_private_name(path.name): return True path = path.parent From cbf1c60f03e3e6be1cd95194337d84db2cac1c93 Mon Sep 17 00:00:00 2001 From: G Roques Date: Thu, 6 Aug 2020 19:07:16 -0500 Subject: [PATCH 19/22] Reword comment --- src/pydocstyle/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index a1ee674d..7449d01f 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -130,7 +130,7 @@ def _is_inside_private_package(self): path = Path(self.name).parent # Ignore the actual module's name syspath = [Path(p) for p in sys.path] # Convert to pathlib.Path. - # 2nd condition is to bail if we are at the root directory or in `PYTHONPATH`. + # Bail if we are at the root directory or in `PYTHONPATH`. while path != path.parent and path not in syspath: if self._is_private_name(path.name): return True From dde693ffd68040fcb46880247e715d9dc9496368 Mon Sep 17 00:00:00 2001 From: G Roques Date: Thu, 6 Aug 2020 19:18:38 -0500 Subject: [PATCH 20/22] Add tests with the same path over different sys.path cases --- src/tests/parser_test.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/tests/parser_test.py b/src/tests/parser_test.py index ced3b653..05a329ec 100644 --- a/src/tests/parser_test.py +++ b/src/tests/parser_test.py @@ -616,6 +616,24 @@ def test_module_publicity_with_private_paths(private_path): assert not 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(): """Test that a complex module is parsed correctly.""" parser = Parser() From 2f6ab9e3fb516e44163d1299303947275cae8027 Mon Sep 17 00:00:00 2001 From: G Roques Date: Mon, 10 Aug 2020 18:45:53 -0500 Subject: [PATCH 21/22] Add note about checking sys.path for determining publicity --- docs/snippets/publicity.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/snippets/publicity.rst b/docs/snippets/publicity.rst index dd165a11..332aeae7 100644 --- a/docs/snippets/publicity.rst +++ b/docs/snippets/publicity.rst @@ -27,6 +27,12 @@ a class called ``_Foo`` is considered private. A method ``bar`` in ``_Foo`` is also considered private since its parent is a private class, even though its name does not begin with a single underscore. +Note, a construct's parent is recursively checked upward until we reach a directory +in ``sys.path`` to avoid considering the complete filepath of a module. +For example, consider the module ``/_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*. + Modules are parsed to look if ``__all__`` is defined. If so, only those top level constructs are considered public. The parser looks for ``__all__`` defined as a literal list or tuple. As the parser doesn't execute the module, From 0e5648c99325554e947e475eb362c249e004925b Mon Sep 17 00:00:00 2001 From: Sambhav Kothari Date: Sat, 22 Aug 2020 18:36:02 +0100 Subject: [PATCH 22/22] Apply suggestions from code review --- docs/snippets/publicity.rst | 2 +- src/pydocstyle/parser.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/snippets/publicity.rst b/docs/snippets/publicity.rst index 332aeae7..e3d5d8cb 100644 --- a/docs/snippets/publicity.rst +++ b/docs/snippets/publicity.rst @@ -27,7 +27,7 @@ a class called ``_Foo`` is considered private. A method ``bar`` in ``_Foo`` is also considered private since its parent is a private class, even though its name does not begin with a single underscore. -Note, a construct's parent is recursively checked upward until we reach a directory +Note, a module's parent is recursively checked upward until we reach a directory in ``sys.path`` to avoid considering the complete filepath of a module. For example, consider the module ``/_foo/bar/baz.py``. If ``PYTHONPATH`` is set to ``/``, then ``baz.py`` is *private*. diff --git a/src/pydocstyle/parser.py b/src/pydocstyle/parser.py index 7449d01f..1b17c36c 100644 --- a/src/pydocstyle/parser.py +++ b/src/pydocstyle/parser.py @@ -127,7 +127,7 @@ def is_public(self): 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 + 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`.