Add config option to skip private functions by ed741 · Pull Request #282 · jsh9/pydoclint

ed741 · 2026-02-16T15:50:48Z

Adds a flag to skip private functions from having their doc-strings linted: skipCheckingPrivateFunctions

This feature would be really great for my team's projects, hopefully you agree it is valuable - if anything isn't right please let me know.

jsh9 · 2026-02-19T00:43:25Z

Hi, thanks for creating this PR!

I asked an AI coding assistant to review your code changes, and here's the suggestions by AI:

PR #282 Review: "Add config option to skip private functions"

Summary

The PR adds a --skip-checking-private-functions / -scpf flag that, when True, skips docstring linting for any function/method whose name starts with _ but is not a dunder (e.g. init).

🐛 Critical Bug: Spurious `from traitlets import default` import

In flake8_entry.py#L8, the diff adds:

from traitlets import default

This is clearly an accidental leftover (likely from IDE autocomplete). traitlets is not a dependency of pydoclint, and default is never used. This will cause an ImportError for any user who doesn't have traitlets installed.

⚠️ Behavior Gap: Name-mangled methods (`__foo`) are not skipped

The skip condition added in visitor.py#L192-L200 is:

node.name.startswith("_") and not node.name.startswith("__")

This means:

_foo → skipped ✅
__init__ → not skipped ✅
__foo__ (dunder) → not skipped ✅
__foo (name-mangled, two leading underscores, no trailing) → not skipped ❌

Name-mangled private methods like __my_method are conventionally "more private" than _foo, yet they would not be skipped by this flag. This is likely unintentional and should at least be documented. The condition should probably be:

node.name.startswith("_") and not (node.name.startswith("__") and node.name.endswith("__"))

⚠️ No documentation update

The PR does not update [docs/config_options.md](https://github.com/jsh9/pydoclint/blob/149d6a79526e0db176f979d0f81d0e6e718a0cf4/docs/config_options.md). Every other option in the codebase has a corresponding entry there (with shortform, default value, and description). This is missing.

🔍 Test coverage gaps

The test case in tests/test_data/edge_cases/33_private_functions/case.py is a good start, but:

No test for __foo (name-mangled) methods — to confirm the current behavior (not skipped) is intentional.
No test for nested private functions — e.g. a private function defined inside another function. The current implementation would skip it when the flag is on, which is probably correct, but it's worth having a test.
Missing spacing: tests/test_edge_cases.py#L635 has 'skipCheckingPrivateFunctions':False missing a space after the colon (minor style nit, inconsistent with the rest of the test file).

✅ What's done well

The core logic in visitor.py#L192-L200 is clean and placed in the right location (before getDocstring is called, consistent with how the existing early-return for non-last constructors works).
The parameter is correctly threaded through main.py → _checkPaths → _checkFile → Visitor.
The default value of False is correct (opt-in behavior, no breaking change).
Dunder methods are correctly excluded from skipping.
The click option in main.py uses type=bool (correct), while the flake8 option uses action='store' with string 'False' default (also correct, consistent with all other flake8 options).

- remove erroneous import - skip name mangled methods - update docs - add test for inner methods

ed741 · 2026-02-19T11:08:11Z

Thanks for the feedback, I've addressed the comments:

Sorry about the import, clearly I've become to reliant on CI to tell me about that stuff.
Docs have been updated.
name mangled / doublelly underscored methods are not skipped properly.
Tests also test that all methods defined inside private methods are also skipped.

jsh9 · 2026-02-20T08:51:04Z

+        if (
+            self.skipCheckingPrivateFunctions
+            and node.name.startswith("_")
+            and not (node.name.startswith("__") and node.name.endswith("__"))


Could you package this logic into a function in https://github.com/jsh9/pydoclint/blob/main/pydoclint/utils/generic.py and add some unit test cases?

This way, you don't need the comment below ("# Skip private functions, but not dunder methods.") because the function name is self explanatory enough.

Done - and added tests in test_generic.py

jsh9 · 2026-02-20T08:51:44Z

+            and node.name.startswith("_")
+            and not (node.name.startswith("__") and node.name.endswith("__"))
+        ):
+            # Skip private functions, but not dunder methods.


Suggested change

# Skip private functions, but not dunder methods.

You don't need this comment if the logic above is packaged into a function with clear naming.

jsh9 · 2026-02-20T08:52:18Z

+
+class TestClass:
+    """
+    Docstring for TestClass, this will still be checked if skip-checking-short-docstrings is False


Suggested change

Docstring for TestClass, this will still be checked if skip-checking-short-docstrings is False

Docstring for TestClass, this will still be checked if

skip-checking-short-docstrings is False

nit: making lines shorter to improve readability

jsh9 · 2026-02-20T08:52:46Z

+    """
+
+    def __init__(self) -> None:
+        """This init will still be checked if skip-checking-short-docstrings is False"""


Suggested change

"""This init will still be checked if skip-checking-short-docstrings is False"""

"""This init will still be checked if

skip-checking-short-docstrings is False"""

nit: making lines shorter to improve readability

jsh9 · 2026-02-20T08:53:21Z

+            bool: Always returns False
+        """
+        def inner_method() -> bool:
+            """inner method should also be skipped as it is inside a private method.


Suggested change

"""inner method should also be skipped as it is inside a private method.

"""inner method should also be skipped as it is inside a

private method.

nit: making lines shorter to improve readability

jsh9 · 2026-02-20T08:54:03Z

+            '33_private_functions/case.py',
+            {
+                'style': 'google',
+                'skipCheckingPrivateFunctions':True,


Suggested change

'skipCheckingPrivateFunctions':True,

'skipCheckingPrivateFunctions': True,

If you run pre-commit run -a on the repo, you can automatically have these formatting issues fixed.

jsh9 · 2026-02-20T08:55:24Z

In addition to my comments above, could you make 2 additional small changes:

Bump version in https://github.com/jsh9/pydoclint/blob/main/pyproject.toml to 0.8.4
Update CHANGELOG (https://github.com/jsh9/pydoclint/blob/main/CHANGELOG.md)

ed741 · 2026-03-13T16:08:28Z

I think that should be all the changes.
pre-commit found a bunch of other fixes thankfully.

ed741 · 2026-03-20T13:21:51Z

It appears updates to muff flagged a whole bunch of tests as improperly formatted. The latest commit fixes this but at the expense of bloating the PR size.

add skipCheckingPrivateFunctions feature

149d6a7

address comments:

10396e5

- remove erroneous import - skip name mangled methods - update docs - add test for inner methods

jsh9 reviewed Feb 20, 2026

View reviewed changes

ed741 added 2 commits March 13, 2026 16:00

address comments and format

a92dfe6

bump version and update changelog

2ca62a0

Make formatting compliant with updated muff

d1a6d22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config option to skip private functions#282

Add config option to skip private functions#282
ed741 wants to merge 5 commits intojsh9:mainfrom
ed741:main

ed741 commented Feb 16, 2026

Uh oh!

jsh9 commented Feb 19, 2026 •

edited

Loading

Uh oh!

ed741 commented Feb 19, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

ed741 Mar 13, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

jsh9 Feb 20, 2026

Uh oh!

jsh9 commented Feb 20, 2026

Uh oh!

ed741 commented Mar 13, 2026

Uh oh!

ed741 commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Docstring for TestClass, this will still be checked if skip-checking-short-docstrings is False
	Docstring for TestClass, this will still be checked if
	skip-checking-short-docstrings is False

	"""This init will still be checked if skip-checking-short-docstrings is False"""
	"""This init will still be checked if
	skip-checking-short-docstrings is False"""

	"""inner method should also be skipped as it is inside a private method.
	"""inner method should also be skipped as it is inside a
	private method.

	'skipCheckingPrivateFunctions':True,
	'skipCheckingPrivateFunctions': True,

Conversation

ed741 commented Feb 16, 2026

Uh oh!

jsh9 commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR #282 Review: "Add config option to skip private functions"

Summary

🐛 Critical Bug: Spurious from traitlets import default import

⚠️ Behavior Gap: Name-mangled methods (__foo) are not skipped

⚠️ No documentation update

🔍 Test coverage gaps

✅ What's done well

Uh oh!

ed741 commented Feb 19, 2026

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

ed741 Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

jsh9 commented Feb 20, 2026

Uh oh!

ed741 commented Mar 13, 2026

Uh oh!

ed741 commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jsh9 commented Feb 19, 2026 •

edited

Loading

🐛 Critical Bug: Spurious `from traitlets import default` import

⚠️ Behavior Gap: Name-mangled methods (`__foo`) are not skipped