From 7976d68ce42e2c9dd3ad2916058be10046df5694 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Tue, 4 Jan 2022 14:46:52 +0100 Subject: [PATCH 1/4] Add automatically generated options --- .github/CONTRIBUTING.md | 6 ++- pydocstringformatter/run.py | 6 ++- pydocstringformatter/utils/__init__.py | 2 + .../utils/argument_parsing.py | 20 +++++++++ tests/test_run.py | 43 +++++++++++++++++-- 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index e87e1578..3fddb2b0 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -8,8 +8,10 @@ Use `pre-commit install` to install the pre-commit hook for the repository. - Implement a Formatter by inheriting from `pydocstringformatter.formatting.Formatter` - Add your new formatter to `pydocstringformatter.formatting.FORMATTERS` -- Choose a proper name because this will be user-facing: the name will be used for - options of the CLI. +- Write a clear docstring because this will be user-facing: it's what will be seen in + the help message for the formatters command line option. +- Choose a proper name because this will be user-facing: the name will be used to turn + the checker on and off via the command line or config files. ### Testing diff --git a/pydocstringformatter/run.py b/pydocstringformatter/run.py index ab1f2aa4..087cf9ec 100644 --- a/pydocstringformatter/run.py +++ b/pydocstringformatter/run.py @@ -9,6 +9,7 @@ from typing import List, Union from pydocstringformatter import __version__, formatting, utils +from pydocstringformatter.formatting import FORMATTERS class _Run: @@ -16,6 +17,7 @@ class _Run: def __init__(self, argv: Union[List[str], None]) -> None: self.arg_parser = utils._register_arguments(__version__) + utils._register_arguments_formatters(self.arg_parser, FORMATTERS) self.config = argparse.Namespace() if argv := argv or sys.argv[1:]: @@ -47,9 +49,11 @@ def _format_file(self, filename: Path) -> bool: for index, tokeninfo in enumerate(tokens): new_tokeninfo = tokeninfo + formatter_options = vars(self.config) if utils._is_docstring(new_tokeninfo, tokens[index - 1]): for formatter in formatting.FORMATTERS: - new_tokeninfo = formatter.treat_token(new_tokeninfo) + if formatter_options[formatter.name]: + new_tokeninfo = formatter.treat_token(new_tokeninfo) changed_tokens.append(new_tokeninfo) if tokeninfo != new_tokeninfo: diff --git a/pydocstringformatter/utils/__init__.py b/pydocstringformatter/utils/__init__.py index fe4dee70..06bdabe9 100644 --- a/pydocstringformatter/utils/__init__.py +++ b/pydocstringformatter/utils/__init__.py @@ -2,6 +2,7 @@ _parse_command_line_arguments, _parse_toml_file, _register_arguments, + _register_arguments_formatters, ) from pydocstringformatter.utils.exceptions import ( ParsingError, @@ -22,6 +23,7 @@ "ParsingError", "PydocstringFormatterError", "_register_arguments", + "_register_arguments_formatters", "TomlParsingError", "_print_to_console", ] diff --git a/pydocstringformatter/utils/argument_parsing.py b/pydocstringformatter/utils/argument_parsing.py index b4ee58eb..356fb79c 100644 --- a/pydocstringformatter/utils/argument_parsing.py +++ b/pydocstringformatter/utils/argument_parsing.py @@ -4,6 +4,7 @@ import tomli +from pydocstringformatter.formatting.base import Formatter from pydocstringformatter.utils.exceptions import TomlParsingError, UnrecognizedOption OPTIONS_TYPES = {"write": "store_true"} @@ -36,7 +37,26 @@ def _register_arguments(version: str) -> argparse.ArgumentParser: version=version, help="Show version number and exit", ) + return parser + +def _register_arguments_formatters( + parser: argparse.ArgumentParser, formatters: List[Formatter] +) -> argparse.ArgumentParser: + """Register a list of formatters, so they can all be deactivated or activated.""" + for formatter in formatters: + name = formatter.name + help_text = f"ctivate the {name} formatter" + parser.add_argument( + f"--{name}", + action="store_true", + dest=name, + default=not formatter.optional, + help=f"A{help_text} : {formatter.__doc__}", + ) + parser.add_argument( + f"--no-{name}", action="store_false", dest=name, help=f"Dea{help_text}" + ) return parser diff --git a/tests/test_run.py b/tests/test_run.py index 03adcd7c..8d96e7f7 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -2,19 +2,26 @@ import os import sys from pathlib import Path +from typing import List +from unittest.mock import patch import pytest import pydocstringformatter +from pydocstringformatter.formatting import FORMATTERS def test_no_arguments(capsys: pytest.CaptureFixture[str]) -> None: """Test that we warn when no arguments are provided""" sys.argv = ["pydocstringformatter"] pydocstringformatter.run_docstring_formatter() - output = capsys.readouterr() - assert output.out.startswith("usage: pydocstringformatter [-h]") - assert not output.err + out, err = capsys.readouterr() + assert out.startswith("usage: pydocstringformatter [-h]") + assert "--beginning-quotes" in out + assert "Activate the beginning-quotes formatter" in out + assert "--no-beginning-quotes" in out + assert "Deactivate the beginning-quotes formatter" in out + assert not err def test_sys_agv_as_arguments( @@ -106,3 +113,33 @@ def test_output_message_two_files( """ ) assert not output.err + + +@pytest.mark.parametrize( + "args,should_format", + [ + [[f"--no-{f.name}" for f in FORMATTERS], False], + [[f"--{f.name}" for f in FORMATTERS], True], + ], +) +def test_optional_formatters( + args: List[str], + should_format: bool, + capsys: pytest.CaptureFixture[str], + tmp_path: Path, +) -> None: + """Test that optional check are activated or not depending on options.""" + bad_docstring = tmp_path / "bad_docstring.py" + bad_docstring.write_text(f'"""{"a" * 120}\n{"b" * 120}"""') + with patch.object(sys, "argv", ["pydocstringformatter", str(bad_docstring)] + args): + pydocstringformatter.run_docstring_formatter() + out, err = capsys.readouterr() + assert not err + if should_format: + msg = "Nothing was modified, but all formatters are activated." + assert "Nothing to do!" not in out + expected = ["---", "@@", "+++"] + assert all(e in out for e in expected), msg + else: + msg = "Something was modified, but all formatter are deactivated." + assert "Nothing to do!" in out, msg From f7399018f3b74020728bcba04f79894798cfadab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 6 Jan 2022 10:04:52 +0100 Subject: [PATCH 2/4] Refactor to avoid using a default value in argparse See discussion here: https://github.com/DanielNoord/pydocstringformatter/pull/15#discussion_r778666929 And here https://github.com/DanielNoord/pydocstringformatter/pull/21 --- .github/CONTRIBUTING.md | 4 +-- pydocstringformatter/run.py | 9 +++-- pydocstringformatter/utils/__init__.py | 6 ++-- .../utils/argument_parsing.py | 35 ++++++++++++++++++- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 3fddb2b0..b683f17b 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -9,9 +9,9 @@ Use `pre-commit install` to install the pre-commit hook for the repository. - Implement a Formatter by inheriting from `pydocstringformatter.formatting.Formatter` - Add your new formatter to `pydocstringformatter.formatting.FORMATTERS` - Write a clear docstring because this will be user-facing: it's what will be seen in - the help message for the formatters command line option. + the help message for the formatter's command line option. - Choose a proper name because this will be user-facing: the name will be used to turn - the checker on and off via the command line or config files. + the formatter on and off via the command line or config files. ### Testing diff --git a/pydocstringformatter/run.py b/pydocstringformatter/run.py index 087cf9ec..a113ec30 100644 --- a/pydocstringformatter/run.py +++ b/pydocstringformatter/run.py @@ -9,7 +9,6 @@ from typing import List, Union from pydocstringformatter import __version__, formatting, utils -from pydocstringformatter.formatting import FORMATTERS class _Run: @@ -17,13 +16,13 @@ class _Run: def __init__(self, argv: Union[List[str], None]) -> None: self.arg_parser = utils._register_arguments(__version__) - utils._register_arguments_formatters(self.arg_parser, FORMATTERS) + utils._register_arguments_formatters(self.arg_parser, formatting.FORMATTERS) self.config = argparse.Namespace() if argv := argv or sys.argv[1:]: - utils._parse_toml_file(self.arg_parser, self.config) - utils._parse_command_line_arguments(self.arg_parser, argv, self.config) - + utils._parse_options( + self.arg_parser, self.config, argv, formatting.FORMATTERS + ) self._check_files(self.config.files) else: self.arg_parser.print_help() diff --git a/pydocstringformatter/utils/__init__.py b/pydocstringformatter/utils/__init__.py index 06bdabe9..cde2b24e 100644 --- a/pydocstringformatter/utils/__init__.py +++ b/pydocstringformatter/utils/__init__.py @@ -1,6 +1,5 @@ from pydocstringformatter.utils.argument_parsing import ( - _parse_command_line_arguments, - _parse_toml_file, + _parse_options, _register_arguments, _register_arguments_formatters, ) @@ -18,12 +17,11 @@ "_find_python_files", "_generate_diff", "_is_docstring", - "_parse_command_line_arguments", - "_parse_toml_file", "ParsingError", "PydocstringFormatterError", "_register_arguments", "_register_arguments_formatters", "TomlParsingError", + "_parse_options", "_print_to_console", ] diff --git a/pydocstringformatter/utils/argument_parsing.py b/pydocstringformatter/utils/argument_parsing.py index 356fb79c..c412add9 100644 --- a/pydocstringformatter/utils/argument_parsing.py +++ b/pydocstringformatter/utils/argument_parsing.py @@ -51,7 +51,6 @@ def _register_arguments_formatters( f"--{name}", action="store_true", dest=name, - default=not formatter.optional, help=f"A{help_text} : {formatter.__doc__}", ) parser.add_argument( @@ -100,3 +99,37 @@ def _parse_toml_file( arguments += _parse_toml_option(key, value) parser.parse_args(arguments, namespace) + + +def _load_formatters_default_option( + parser: argparse.ArgumentParser, + namespace: argparse.Namespace, + formatters: List[Formatter], +) -> None: + """Load the list of formatters based on their 'optional' attribute.""" + arguments: List[str] = [] + for formatter in formatters: + if formatter.optional: + arguments.append(f"--no-{formatter.name}") + elif not formatter.optional: + arguments.append(f"--{formatter.name}") + + parser.parse_known_args(arguments, namespace) + + +def _parse_options( + parser: argparse.ArgumentParser, + namespace: argparse.Namespace, + argv: List[str], + formatters: List[Formatter], +) -> None: + """Load all default option values. + + The order of parsing is: + 1. default values, 2. configuration files, 3. command line arguments. + """ + _load_formatters_default_option(parser, namespace, formatters) + + _parse_toml_file(parser, namespace) + + _parse_command_line_arguments(parser, argv, namespace) From ebb339730522f610ab5e659f8393478c2cb64013 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 8 Jan 2022 14:57:41 +0100 Subject: [PATCH 3/4] Simplification of the use of namespace --- pydocstringformatter/run.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pydocstringformatter/run.py b/pydocstringformatter/run.py index a113ec30..60178634 100644 --- a/pydocstringformatter/run.py +++ b/pydocstringformatter/run.py @@ -48,10 +48,9 @@ def _format_file(self, filename: Path) -> bool: for index, tokeninfo in enumerate(tokens): new_tokeninfo = tokeninfo - formatter_options = vars(self.config) if utils._is_docstring(new_tokeninfo, tokens[index - 1]): for formatter in formatting.FORMATTERS: - if formatter_options[formatter.name]: + if getattr(self.config, formatter.name): new_tokeninfo = formatter.treat_token(new_tokeninfo) changed_tokens.append(new_tokeninfo) From 38a496738adb21f68992c4b3c9796237a13a7c21 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Sat, 8 Jan 2022 15:15:50 +0100 Subject: [PATCH 4/4] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> --- pydocstringformatter/utils/argument_parsing.py | 2 +- tests/test_run.py | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pydocstringformatter/utils/argument_parsing.py b/pydocstringformatter/utils/argument_parsing.py index c412add9..38d76b1e 100644 --- a/pydocstringformatter/utils/argument_parsing.py +++ b/pydocstringformatter/utils/argument_parsing.py @@ -106,7 +106,7 @@ def _load_formatters_default_option( namespace: argparse.Namespace, formatters: List[Formatter], ) -> None: - """Load the list of formatters based on their 'optional' attribute.""" + """Parse the state of the list of formatters based on their 'optional' attribute.""" arguments: List[str] = [] for formatter in formatters: if formatter.optional: diff --git a/tests/test_run.py b/tests/test_run.py index 8d96e7f7..02d76c2e 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -3,7 +3,6 @@ import sys from pathlib import Path from typing import List -from unittest.mock import patch import pytest @@ -12,11 +11,13 @@ def test_no_arguments(capsys: pytest.CaptureFixture[str]) -> None: - """Test that we warn when no arguments are provided""" + """Test that we display a help message when no arguments are provided.""" sys.argv = ["pydocstringformatter"] pydocstringformatter.run_docstring_formatter() out, err = capsys.readouterr() assert out.startswith("usage: pydocstringformatter [-h]") + + # Test that we print help messages for individual formatters as well assert "--beginning-quotes" in out assert "Activate the beginning-quotes formatter" in out assert "--no-beginning-quotes" in out @@ -128,11 +129,10 @@ def test_optional_formatters( capsys: pytest.CaptureFixture[str], tmp_path: Path, ) -> None: - """Test that optional check are activated or not depending on options.""" + """Test that (optional) formatters are activated or not depending on options.""" bad_docstring = tmp_path / "bad_docstring.py" bad_docstring.write_text(f'"""{"a" * 120}\n{"b" * 120}"""') - with patch.object(sys, "argv", ["pydocstringformatter", str(bad_docstring)] + args): - pydocstringformatter.run_docstring_formatter() + pydocstringformatter.run_docstring_formatter([str(bad_docstring)] + args) out, err = capsys.readouterr() assert not err if should_format: