From 6112943ca3f2a74c0419d8d2452884dd144c8309 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 15:48:39 -0600 Subject: [PATCH 01/12] chore: Switch to Ruff Use Ruff to replace a variety of linters/formatters. --- .pre-commit-config.yaml | 40 ++++---------------- CONTRIBUTING.md | 41 +++----------------- README.md | 8 ++-- doc/source/index.rst | 12 ++---- pyproject.toml | 56 +++++++++++++++++++++------- reverse_argparse/reverse_argparse.py | 2 +- test/requirements.txt | 6 +-- 7 files changed, 63 insertions(+), 102 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a4fab0e..24fc739 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,6 +6,13 @@ ci: repos: + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.4.1 + hooks: + - id: ruff + args: [ --fix ] + - id: ruff-format + - repo: https://github.com/commitizen-tools/commitizen rev: v3.22.0 hooks: @@ -38,44 +45,11 @@ repos: - id: rst-directive-colons - id: rst-inline-touching-normal - - repo: https://github.com/psf/black - rev: 24.4.0 - hooks: - - id: black - - - repo: https://github.com/PyCQA/bandit - rev: 1.7.8 - hooks: - - id: bandit - args: ["-c", "pyproject.toml"] - additional_dependencies: ["bandit[toml]"] - - repo: https://github.com/PyCQA/doc8 rev: v1.1.1 hooks: - id: doc8 - - repo: https://github.com/PyCQA/flake8 - rev: 7.0.0 - hooks: - - id: flake8 - - - repo: https://github.com/PyCQA/isort - rev: 5.13.2 - hooks: - - id: isort - - - repo: https://github.com/PyCQA/prospector - rev: v1.10.3 - hooks: - - id: prospector - additional_dependencies: ["dateparser"] - - - repo: https://github.com/PyCQA/pydocstyle - rev: 6.3.0 - hooks: - - id: pydocstyle - - repo: https://github.com/regebro/pyroma rev: "4.2" hooks: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8a5901f..f5b4abe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,6 +69,7 @@ pre-commit install --hook-type commit-msg --hook-type pre-push [precommit]: https://pre-commit.com/ The checks we perform are the following: +* Use [ruff][ruff] to lint and format the code and docstrings. * Use [commitizen][commitizen] to ensure commit messages match the [Conventional Commits specification][conventional]. Use the [Conventional Commits extension for VS Code][extension] (or something @@ -84,42 +85,18 @@ The checks we perform are the following: * Trim trailing whitespace. * Ensure we use [type-hinting][typing]. * Check for common mistakes in [reStructuredText][rest] in our documentation. -* Use [black][black] to automatically format the code. -* Use [Bandit][bandit] to find common security issues. * Use [doc8][doc8] to enforce our style for our documentation. -* Lint the code with [flake8][flake8]. -* Use [isort][isort] to ensure `import` statements are sorted correctly. -* Use [prospector][prospector] to check for various errors, potential problems, - convention violations, complexity, etc. This uses the following tools under - the hood: - * [dodgy][dodgy] - * [mccabe][mccabe] - * [pycodestyle][pycodestyle] - * [Pyflakes][pyflakes] - * [Pylint][pylint] -* Use [pydocstyle][pydocstyle] to ensure our docstrings line up with - [PEP 257][pep257]. * Use [pyroma][pyroma] to ensure our package complies with the best practices of the Python packaging ecosystem. +[ruff]: https://docs.astral.sh/ruff/ [commitizen]: https://github.com/commitizen-tools/commitizen [conventional]: https://www.conventionalcommits.org/en/v1.0.0/ [extension]: https://marketplace.visualstudio.com/items?itemName=vivaxy.vscode-conventional-commits [mypy]: https://github.com/python/mypy [typing]: https://docs.python.org/3/library/typing.html -[black]: https://github.com/psf/black -[bandit]: https://github.com/PyCQA/bandit +[rest]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html [doc8]: https://github.com/PyCQA/doc8 -[flake8]: https://github.com/pyCQA/flake8 -[isort]: https://github.com/pyCQA/isort -[prospector]: https://github.com/landscapeio/prospector -[dodgy]: https://github.com/landscapeio/dodgy -[mccabe]: https://github.com/PyCQA/mccabe -[pycodestyle]: https://pycodestyle.pycqa.org/en/latest/ -[pyflakes]: https://launchpad.net/pyflakes -[pylint]: https://www.pylint.org/ -[pydocstyle]: https://github.com/PyCQA/pydocstyle -[pep257]: http://www.python.org/dev/peps/pep-0257/ [pyroma]: https://github.com/regebro/pyroma ### VS Code @@ -183,9 +160,8 @@ search for and install them. These are the ones we recommend: editor. * **autoDocstring - Python Docstring Generator:** Quickly generate docstrings for Python functions. -* **Flake8:** Linting support for Python files using flake8. +* **Mpy Type Checker:** Type checking support for Python. * **Pylance:** Fast, feature-rich language support for Python. -* **Pylint:** Linting support for Python files. * **Pytest IntelliSense:** Adds IntelliSense support for [pytest][pytest] fixtures. * **Python:** Rich support for the Python language. @@ -194,6 +170,7 @@ search for and install them. These are the ones we recommend: or [testplan][testplan] tests with the Test Explorer UI (see **General** above). * **Python Type Hint:** Type hint autocompletion. +* **Ruff:** Automatic linting and formatting. * **Sourcery:** Automatic code review and refactoring. [unittest]: https://docs.python.org/3/library/unittest.html @@ -233,13 +210,6 @@ After installing the various extensions, you'll also want to customize your * **Terminal Git Editor:** Check. * pre-commit-helper * **Run On Save:** Select "all hooks". - * Python - * **Formatting:** Provider: Select "black". - * **Linting: Bandit Enabled:** Check. - * **Linting: Flake8 Enabled:** Check. - * **Linting: Lint On Save:** Check. - * **Linting: Mypy Enabled:** Check. - * **Linting: Prospector Enabled:** Check. * Python Docstring Generator configuration * **Docstring Format:** Select "google-notypes". * **Start On New Line:** Check. @@ -372,7 +342,6 @@ utilize [type-hinting][typing] wherever possible for clarity's sake. [docstrings]: https://www.python.org/dev/peps/pep-0257 [google]: https://github.com/google/styleguide/blob/gh-pages/pyguide.md#38-comments-and-docstrings -[rest]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html [docs]: https://reverse-argparse.readthedocs.io [sphinx]: https://www.sphinx-doc.org/en/master/ diff --git a/README.md b/README.md index 30207b9..b8da399 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,3 @@ -[![Code Style: black](https://img.shields.io/badge/Code%20Style-black-000000.svg)](https://github.com/psf/black) [![codecov](https://codecov.io/gh/sandialabs/reverse_argparse/branch/master/graph/badge.svg?token=FmDStZ6FVR)](https://codecov.io/gh/sandialabs/reverse_argparse) [![CodeFactor](https://www.codefactor.io/repository/github/sandialabs/reverse_argparse/badge/master)](https://www.codefactor.io/repository/github/sandialabs/reverse_argparse/overview/master) [![CodeQL](https://github.com/sandialabs/reverse_argparse/actions/workflows/github-code-scanning/codeql/badge.svg)](https://github.com/sandialabs/reverse_argparse/actions/workflows/github-code-scanning/codeql) @@ -8,18 +7,17 @@ [![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md) [![GitHub contributors](https://img.shields.io/github/contributors/sandialabs/reverse_argparse.svg)](https://github.com/sandialabs/reverse_argparse/graphs/contributors) [![Documentation Status](https://readthedocs.org/projects/reverse-argparse/badge/?version=latest)](https://reverse-argparse.readthedocs.io/en/latest/?badge=latest) -[![Anaconda-Server Badge](https://anaconda.org/conda-forge/reverse-argparse/badges/license.svg)](LICENSE.md) -[![Linting: Pylint](https://img.shields.io/badge/Linting-Pylint-yellowgreen)](https://github.com/pylint-dev/pylint) +[![License](https://anaconda.org/conda-forge/reverse-argparse/badges/license.svg)](LICENSE.md) [![Merged PRs](https://img.shields.io/github/issues-pr-closed-raw/sandialabs/reverse_argparse.svg?label=merged+PRs)](https://github.com/sandialabs/reverse_argparse/pulls?q=is:pr+is:merged) [![OpenSSF Best Practices](https://bestpractices.coreinfrastructure.org/projects/7632/badge)](https://bestpractices.coreinfrastructure.org/projects/7632) [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/sandialabs/reverse_argparse/badge)](https://securityscorecards.dev/viewer/?uri=github.com/sandialabs/reverse_argparse) -![Anaconda-Server Badge](https://anaconda.org/conda-forge/reverse-argparse/badges/platforms.svg) +![Platforms](https://anaconda.org/conda-forge/reverse-argparse/badges/platforms.svg) [![pre-commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) [![pre-commit.ci Status](https://results.pre-commit.ci/badge/github/sandialabs/reverse_argparse/master.svg)](https://results.pre-commit.ci/latest/github/sandialabs/reverse_argparse/master) [![PyPI - Version](https://img.shields.io/pypi/v/reverse-argparse?label=PyPI)](https://pypi.org/project/reverse-argparse/) ![PyPI - Downloads](https://img.shields.io/pypi/dm/reverse-argparse?label=PyPI%20downloads) ![Python Version](https://img.shields.io/badge/Python-3.8|3.9|3.10|3.11|3.12-blue.svg) -[![Security: Bandit](https://img.shields.io/badge/Security-Bandit-yellow.svg)](https://github.com/PyCQA/bandit) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) # reverse_argparse diff --git a/doc/source/index.rst b/doc/source/index.rst index 23ca0e1..23cb67d 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -10,7 +10,6 @@ reverse_argparse examples reference -|Code Style: black| |codecov| |CodeFactor| |CodeQL| @@ -21,7 +20,6 @@ reverse_argparse |GitHub Contributors| |Documentation Status| |License| -|Linting: Pylint| |Merged PRs| |OpenSSF Best Practices| |OpenSSF Scorecard| @@ -31,10 +29,8 @@ reverse_argparse |PyPI Version| |PyPI Downloads| |Python Version| -|Security: Bandit| +|Ruff| -.. |Code Style: black| image:: https://img.shields.io/badge/Code%20Style-black-000000.svg - :target: https://github.com/psf/black .. |codecov| image:: https://codecov.io/gh/sandialabs/reverse_argparse/branch/master/graph/badge.svg?token=FmDStZ6FVR :target: https://codecov.io/gh/sandialabs/reverse_argparse .. |CodeFactor| image:: https://www.codefactor.io/repository/github/sandialabs/reverse_argparse/badge/master @@ -54,8 +50,6 @@ reverse_argparse :target: https://reverse-argparse.readthedocs.io/en/latest/?badge=latest .. |License| image:: https://anaconda.org/conda-forge/reverse-argparse/badges/license.svg :target: https://github.com/sandialabs/reverse_argparse/blob/master/LICENSE.md -.. |Linting: Pylint| image:: https://img.shields.io/badge/Linting-Pylint-yellowgreen - :target: https://github.com/pylint-dev/pylint .. |Merged PRs| image:: https://img.shields.io/github/issues-pr-closed-raw/sandialabs/reverse_argparse.svg?label=merged+PRs :target: https://github.com/sandialabs/reverse_argparse/pulls?q=is:pr+is:merged .. |OpenSSF Best Practices| image:: https://bestpractices.coreinfrastructure.org/projects/7632/badge @@ -71,8 +65,8 @@ reverse_argparse :target: https://pypi.org/project/reverse-argparse/ .. |PyPI Downloads| image:: https://img.shields.io/pypi/dm/reverse-argparse?label=PyPI%20downloads .. |Python Version| image:: https://img.shields.io/badge/Python-3.8|3.9|3.10|3.11|3.12-blue.svg -.. |Security: Bandit| image:: https://img.shields.io/badge/Security-Bandit-yellow.svg - :target: https://github.com/PyCQA/bandit +.. |Ruff| image:: https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json + :target: https://github.com/astral-sh/ruff The ``reverse_argparse`` module provides a means of undoing the argument parsing provided by :mod:`argparse`. That is, it can take a diff --git a/pyproject.toml b/pyproject.toml index 49c3c7b..f7b2c7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,18 +3,10 @@ requires = ["poetry-core"] build-backend = "poetry.core.masonry.api" -[pydocstyle] -convention = "google" - - [tool.bandit.assert_used] skips = ["**/test_*.py"] -[tool.black] -line-length = 79 - - [tool.commitizen] name = "cz_customize" @@ -23,11 +15,6 @@ name = "cz_customize" schema_pattern = "(?s)(build|chore|ci|docs|feat|fix|minor|patch|perf|refactor|style|test|revert)(\\(\\S+\\))?!?:( [^\\n\\r]+)((\\n\\n.*)|(\\s*))?$" -[tool.isort] -profile = "black" -line_length = 79 - - [tool.poetry] name = "reverse_argparse" version = "1.0.6" @@ -77,6 +64,49 @@ python = ">=3.8" # list of Poetry development dependencies. +[tool.ruff] +line-length = 79 + + +[tool.ruff.lint] +extend-select = [ + "A", + "B", + "BLE", + "C4", + "C90", + "D", + "DTZ", + "E", + "EM", + "ERA", + "EXE", + "FBT", + "NPY", + "PGH", + "PL", + "PT", + "PTH", + "RET", + "RSE", + "RUF", + "S", + "SIM", + "TID", + "TCH", + "TRY", + "UP", + "W", +] +ignore = [ + "D212", +] + + +[tool.ruff.lint.pydocstyle] +convention = "google" + + [tool.semantic_release] build_command = "python3 -m pip install poetry && poetry build" commit_message = """ diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index 62f00dd..2106b2d 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -91,7 +91,7 @@ def _unparse_args(self) -> None: self._unparse_action(action) self._unparsed[-1] = True - def _unparse_action(self, action: Action) -> None: + def _unparse_action(self, action: Action) -> None: # noqa: C901 """ Unparse a single action. diff --git a/test/requirements.txt b/test/requirements.txt index 4585c8b..9d956b9 100644 --- a/test/requirements.txt +++ b/test/requirements.txt @@ -1,14 +1,10 @@ # Requirements for testing `reverse_argparse`. -bandit -black dateparser -flake8 -flake8-bugbear mypy pre-commit -prospector pyroma pytest pytest-cov pytest-mock +ruff From f0b7eb96cff1cd6a6360b1e9eba0b66c614c6ac1 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 11:58:09 -0600 Subject: [PATCH 02/12] docs: Add docstrings to test/example files --- example/test_examples.py | 6 ++++++ test/test_reverse_argparse.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/example/test_examples.py b/example/test_examples.py index d2c46dd..bc79fee 100755 --- a/example/test_examples.py +++ b/example/test_examples.py @@ -15,6 +15,7 @@ def test_basic() -> None: + """Ensure ``basic.py`` produces the expected results.""" example = Path(__file__).parent / "basic.py" result = subprocess.run( [example, "--foo", "bar"], @@ -32,6 +33,7 @@ def test_basic() -> None: def test_default_values() -> None: + """Ensure ``default_values.py`` produces the expected results.""" example = Path(__file__).parent / "default_values.py" result = subprocess.run( [example, "--foo", "bar"], @@ -49,6 +51,7 @@ def test_default_values() -> None: def test_relative_references() -> None: + """Ensure ``relative_references.py`` produces the expected results.""" example = Path(__file__).parent / "relative_references.py" result = subprocess.run( [example, "--src", "bar.txt"], @@ -67,6 +70,7 @@ def test_relative_references() -> None: def test_post_processing() -> None: + """Ensure ``post_processing.py`` produces the expected results.""" example = Path(__file__).parent / "post_processing.py" result = subprocess.run( [example, "--before", "'30 minutes ago'"], @@ -89,6 +93,7 @@ def test_post_processing() -> None: def test_pretty_printing() -> None: + """Ensure ``pretty_printing.py`` produces the expected results.""" example = Path(__file__).parent / "pretty_printing.py" result = subprocess.run( [example, "--foo", "eggs", "--src", "file.txt", "--before", "'today'"], @@ -116,6 +121,7 @@ def test_pretty_printing() -> None: def test_subparsers() -> None: + """Ensure ``subparsers.py`` produces the expected results.""" example = Path(__file__).parent / "subparsers.py" result = subprocess.run( [example, "foo", "--one", "eggs"], diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index 494fffd..72534b2 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -21,6 +21,12 @@ @pytest.fixture() def parser() -> ArgumentParser: + """ + Pre-populate an ``ArgumentParser`` with a variety of options. + + Returns: + The ``ArgumentParser`` to be used in a number of tests. + """ p = ArgumentParser() p.add_argument("pos1", nargs="*") p.add_argument("pos2") @@ -107,6 +113,7 @@ def strip_first_entry(input: str) -> str: def test_strip_first_entry() -> None: + """Ensure :func:`strip_first_entry` works as expected.""" assert strip_first_entry("foo bar baz") == "bar baz" @@ -124,11 +131,13 @@ def strip_first_line(input: str) -> str: def test_strip_first_line() -> None: + """Ensure :func:`strip_first_line` works as expected.""" assert strip_first_line("foo\nbar\nbaz") == "bar\nbaz" @pytest.mark.parametrize("args", COMPLETE_ARGS) def test_get_effective_command_line_invocation(parser, args) -> None: + """Ensure :func:`get_effective_command_line_invoation` works.""" namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) expected = ( @@ -151,6 +160,7 @@ def test_get_effective_command_line_invocation(parser, args) -> None: @pytest.mark.parametrize("args", COMPLETE_ARGS) def test_get_pretty_command_line_invocation(parser, args) -> None: + """Ensure :func:`get_pretty_command_line_invoation` works as expected.""" namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) expected = """ --opt1 opt1-val \\ @@ -179,6 +189,7 @@ def test_get_pretty_command_line_invocation(parser, args) -> None: def test_get_command_line_invocation_strip_spaces() -> None: + """Ensure extraneous spaces are stripped.""" parser = ArgumentParser() namespace = Namespace() unparser = ReverseArgumentParser(parser, namespace) @@ -233,6 +244,7 @@ def test_get_command_line_invocation_strip_spaces() -> None: ], ) def test__unparse_args(add_args, add_kwargs, args, expected) -> None: + """Ensure :func:`_unparse_args` works as expected.""" parser = ArgumentParser() parser.add_argument(*add_args, **add_kwargs) try: @@ -249,6 +261,12 @@ def test__unparse_args(add_args, add_kwargs, args, expected) -> None: def test__unparse_args_boolean_optional_action() -> None: + """ + Ensure :func:`_unparse_args` works as expected. + + With a ``BooleanOptionalAction``, which became available in Python + 3.9. + """ if sys.version_info.minor >= 9: parser = ArgumentParser() parser.add_argument("--foo", action=BooleanOptionalAction) @@ -262,6 +280,7 @@ def test__unparse_args_boolean_optional_action() -> None: def test__unparse_args_already_unparsed() -> None: + """Ensure unparsing is a no-op if the args have already been unparsed.""" parser = ArgumentParser() namespace = Namespace() unparser = ReverseArgumentParser(parser, namespace) @@ -273,6 +292,7 @@ def test__unparse_args_already_unparsed() -> None: def test__arg_is_default_and_help_is_suppressed() -> None: + """Ensure defaults for suppressed args are suppressed.""" parser = ArgumentParser() parser.add_argument("--suppressed", default=10, help=SUPPRESS) namespace = parser.parse_args(shlex.split("")) @@ -292,6 +312,7 @@ def test__arg_is_default_and_help_is_suppressed() -> None: ], ) def test__get_long_option_strings(strings, expected) -> None: + """Ensure the long-form option is selected from a list.""" unparser = ReverseArgumentParser(ArgumentParser(), Namespace()) assert unparser._get_long_option_strings(strings) == expected @@ -305,6 +326,7 @@ def test__get_long_option_strings(strings, expected) -> None: ], ) def test__get_short_option_strings(strings, expected) -> None: + """Ensure the short-form option is selected from a list.""" unparser = ReverseArgumentParser(ArgumentParser(), Namespace()) assert unparser._get_short_option_strings(strings) == expected @@ -318,6 +340,7 @@ def test__get_short_option_strings(strings, expected) -> None: ], ) def test__get_option_string(strings, expected) -> None: + """Ensure long-form options are preferred over short-form ones.""" parser = ArgumentParser() action = parser.add_argument(*strings) unparser = ReverseArgumentParser(parser, Namespace()) @@ -333,6 +356,7 @@ def test__get_option_string(strings, expected) -> None: ], ) def test__get_option_string_prefer_short(strings, expected) -> None: + """Ensure short-form options are preferred over long-form ones.""" parser = ArgumentParser() action = parser.add_argument(*strings) unparser = ReverseArgumentParser(parser, Namespace()) @@ -358,6 +382,7 @@ def test__get_option_string_prefer_short(strings, expected) -> None: ], ) def test__unparse_store_action(add_args, add_kwargs, args, expected) -> None: + """Ensure ``store`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument(*add_args, **add_kwargs) namespace = parser.parse_args(shlex.split(args)) @@ -393,6 +418,7 @@ def test__unparse_store_action(add_args, add_kwargs, args, expected) -> None: def test__unparse_store_const_action( add_args, add_kwargs, args, expected ) -> None: + """Ensure ``store_const`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument(*add_args, **add_kwargs) namespace = parser.parse_args(shlex.split(args)) @@ -405,6 +431,7 @@ def test__unparse_store_const_action( "args, expected", [(shlex.split("--foo"), " --foo"), ([], None)] ) def test__unparse_store_true_action(args, expected) -> None: + """Ensure ``store_true`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument("--foo", action="store_true") namespace = parser.parse_args(args) @@ -417,6 +444,7 @@ def test__unparse_store_true_action(args, expected) -> None: "args, expected", [(shlex.split("--foo"), " --foo"), ([], None)] ) def test__unparse_store_false_action(args, expected) -> None: + """Ensure ``store_false`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument("--foo", action="store_false") namespace = parser.parse_args(args) @@ -443,6 +471,7 @@ def test__unparse_store_false_action(args, expected) -> None: ], ) def test__unparse_append_action(add_args, add_kwargs, args, expected) -> None: + """Ensure ``append`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument(*add_args, **add_kwargs) namespace = parser.parse_args(shlex.split(args)) @@ -455,6 +484,7 @@ def test__unparse_append_action(add_args, add_kwargs, args, expected) -> None: "args, expected", [("--foo", " --foo"), ("", None)] ) def test__unparse_append_const_action(args, expected) -> None: + """Ensure ``append_const`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument( "--foo", dest="append_const", action="append_const", const=42 @@ -489,6 +519,7 @@ def test__unparse_append_const_action(args, expected) -> None: ], ) def test__unparse_count_action(add_args, add_kwargs, args, expected) -> None: + """Ensure ``count`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument(*add_args, **add_kwargs) namespace = parser.parse_args(shlex.split(args)) @@ -509,6 +540,7 @@ def test__unparse_count_action(add_args, add_kwargs, args, expected) -> None: ], ) def test__unparse_sub_parsers_action(args, expected, pretty) -> None: + """Ensure subparsers are handled appropriately.""" parser = ArgumentParser() parser.add_argument("--foo", action="store_true", help="foo help") subparsers = parser.add_subparsers(help="sub-command help") @@ -528,6 +560,7 @@ def test__unparse_sub_parsers_action(args, expected, pretty) -> None: def test__unparse_sub_parsers_action_nested() -> None: + """Ensure nested subparsers are handled appropriately.""" parser = ArgumentParser() parser.add_argument("--optional-1", action="store_true") parser.add_argument("--optional-2") @@ -569,6 +602,7 @@ def test__unparse_sub_parsers_action_nested() -> None: def test__unparse_extend_action() -> None: + """Ensure ``extend`` actions are handled appropriately.""" parser = ArgumentParser() action = parser.add_argument("--foo", action="extend", nargs="*") namespace = parser.parse_args(shlex.split("--foo bar --foo baz bif")) @@ -593,6 +627,7 @@ def test__unparse_extend_action() -> None: ], ) def test__unparse_boolean_optional_action(default, args, expected) -> None: + """Ensure ``BooleanOptionalAction`` actions are handled appropriately.""" if sys.version_info.minor >= 9: parser = ArgumentParser() action = parser.add_argument( From f18ece4a69291b96b84a80bd8eea0a5f51697026 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:45:16 -0600 Subject: [PATCH 03/12] chore: Ignore security findings in tests/examples --- example/test_examples.py | 54 ++++++++++++++++------------ test/test_reverse_argparse.py | 68 +++++++++++++++++++++-------------- 2 files changed, 73 insertions(+), 49 deletions(-) diff --git a/example/test_examples.py b/example/test_examples.py index bc79fee..fb5d557 100755 --- a/example/test_examples.py +++ b/example/test_examples.py @@ -18,12 +18,12 @@ def test_basic() -> None: """Ensure ``basic.py`` produces the expected results.""" example = Path(__file__).parent / "basic.py" result = subprocess.run( - [example, "--foo", "bar"], + [example, "--foo", "bar"], # noqa: S603 capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 result.stdout == """ The effective command line invocation was: @@ -36,12 +36,12 @@ def test_default_values() -> None: """Ensure ``default_values.py`` produces the expected results.""" example = Path(__file__).parent / "default_values.py" result = subprocess.run( - [example, "--foo", "bar"], + [example, "--foo", "bar"], # noqa: S603 capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 result.stdout == """ The effective command line invocation was: @@ -54,31 +54,31 @@ def test_relative_references() -> None: """Ensure ``relative_references.py`` produces the expected results.""" example = Path(__file__).parent / "relative_references.py" result = subprocess.run( - [example, "--src", "bar.txt"], + [example, "--src", "bar.txt"], # noqa: S603 capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 """ The effective command line invocation was: relative_references.py --bar spam --baz 42 --src """.strip() in result.stdout ) - assert re.search(r"--src /\S+/bar\.txt", result.stdout) + assert re.search(r"--src /\S+/bar\.txt", result.stdout) # noqa: S101 def test_post_processing() -> None: """Ensure ``post_processing.py`` produces the expected results.""" example = Path(__file__).parent / "post_processing.py" result = subprocess.run( - [example, "--before", "'30 minutes ago'"], + [example, "--before", "'30 minutes ago'"], # noqa: S603 capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 """ The effective command line invocation was: post_processing.py --bar spam --baz 42 --before @@ -89,19 +89,29 @@ def test_post_processing() -> None: time_from_example = datetime.strptime( shlex.split(result.stdout)[-1], "%Y-%m-%d %H:%M:%S.%f" ) - assert thirty_miutes_ago - time_from_example < timedelta(seconds=1) + assert ( # noqa: S101 + thirty_miutes_ago - time_from_example < timedelta(seconds=1) + ) def test_pretty_printing() -> None: """Ensure ``pretty_printing.py`` produces the expected results.""" example = Path(__file__).parent / "pretty_printing.py" result = subprocess.run( - [example, "--foo", "eggs", "--src", "file.txt", "--before", "'today'"], + [ # noqa: S603 + example, + "--foo", + "eggs", + "--src", + "file.txt", + "--before", + "'today'", + ], capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 """ The effective command line invocation was: pretty_printing.py \\ @@ -111,25 +121,25 @@ def test_pretty_printing() -> None: """.strip() in result.stdout ) - assert re.search(r"--src /\S+/file\.txt", result.stdout) + assert re.search(r"--src /\S+/file\.txt", result.stdout) # noqa: S101 today = datetime.now() time_from_example = datetime.strptime( shlex.split(result.stdout.splitlines()[-1])[-1], "%Y-%m-%d %H:%M:%S.%f", ) - assert today - time_from_example < timedelta(seconds=1) + assert today - time_from_example < timedelta(seconds=1) # noqa: S101 def test_subparsers() -> None: """Ensure ``subparsers.py`` produces the expected results.""" example = Path(__file__).parent / "subparsers.py" result = subprocess.run( - [example, "foo", "--one", "eggs"], + [example, "foo", "--one", "eggs"], # noqa: S603 capture_output=True, check=True, text=True, - ) # nosec B603 - assert ( + ) + assert ( # noqa: S101 result.stdout == """ The effective command line invocation was: diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index 72534b2..a036ec8 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -114,7 +114,7 @@ def strip_first_entry(input: str) -> str: def test_strip_first_entry() -> None: """Ensure :func:`strip_first_entry` works as expected.""" - assert strip_first_entry("foo bar baz") == "bar baz" + assert strip_first_entry("foo bar baz") == "bar baz" # noqa: S101 def strip_first_line(input: str) -> str: @@ -132,7 +132,7 @@ def strip_first_line(input: str) -> str: def test_strip_first_line() -> None: """Ensure :func:`strip_first_line` works as expected.""" - assert strip_first_line("foo\nbar\nbaz") == "bar\nbaz" + assert strip_first_line("foo\nbar\nbaz") == "bar\nbaz" # noqa: S101 @pytest.mark.parametrize("args", COMPLETE_ARGS) @@ -155,7 +155,7 @@ def test_get_effective_command_line_invocation(parser, args) -> None: result = strip_first_entry( unparser.get_effective_command_line_invocation() ) - assert result == expected + assert result == expected # noqa: S101 @pytest.mark.parametrize("args", COMPLETE_ARGS) @@ -185,7 +185,7 @@ def test_get_pretty_command_line_invocation(parser, args) -> None: expected += """\n pos1-val1 pos1-val2 \\ pos2-val""" result = strip_first_line(unparser.get_pretty_command_line_invocation()) - assert result == expected + assert result == expected # noqa: S101 def test_get_command_line_invocation_strip_spaces() -> None: @@ -196,11 +196,13 @@ def test_get_command_line_invocation_strip_spaces() -> None: unparser._args = ["program_name", " --foo", " ", " --bar"] unparser._unparsed = [True] expected = "program_name --foo --bar" - assert unparser.get_effective_command_line_invocation() == expected + assert ( # noqa: S101 + unparser.get_effective_command_line_invocation() == expected + ) expected = """ --foo \\ --bar""" result = strip_first_line(unparser.get_pretty_command_line_invocation()) - assert result == expected + assert result == expected # noqa: S101 @pytest.mark.parametrize( @@ -257,7 +259,7 @@ def test__unparse_args(add_args, add_kwargs, args, expected) -> None: unparser._unparse_args() else: unparser._unparse_args() - assert unparser._args[1:] == expected + assert unparser._args[1:] == expected # noqa: S101 def test__unparse_args_boolean_optional_action() -> None: @@ -276,7 +278,7 @@ def test__unparse_args_boolean_optional_action() -> None: namespace = Namespace() unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_args() - assert unparser._args[1:] == [" --foo"] + assert unparser._args[1:] == [" --foo"] # noqa: S101 def test__unparse_args_already_unparsed() -> None: @@ -288,7 +290,7 @@ def test__unparse_args_already_unparsed() -> None: args_before = unparser._args.copy() unparser._unparsed = [True] unparser._unparse_args() - assert unparser._args == args_before + assert unparser._args == args_before # noqa: S101 def test__arg_is_default_and_help_is_suppressed() -> None: @@ -300,7 +302,7 @@ def test__arg_is_default_and_help_is_suppressed() -> None: result = strip_first_entry( unparser.get_effective_command_line_invocation() ) - assert result == "" + assert result == "" # noqa: S101 @pytest.mark.parametrize( @@ -314,7 +316,7 @@ def test__arg_is_default_and_help_is_suppressed() -> None: def test__get_long_option_strings(strings, expected) -> None: """Ensure the long-form option is selected from a list.""" unparser = ReverseArgumentParser(ArgumentParser(), Namespace()) - assert unparser._get_long_option_strings(strings) == expected + assert unparser._get_long_option_strings(strings) == expected # noqa: S101 @pytest.mark.parametrize( @@ -328,7 +330,9 @@ def test__get_long_option_strings(strings, expected) -> None: def test__get_short_option_strings(strings, expected) -> None: """Ensure the short-form option is selected from a list.""" unparser = ReverseArgumentParser(ArgumentParser(), Namespace()) - assert unparser._get_short_option_strings(strings) == expected + assert ( # noqa: S101 + unparser._get_short_option_strings(strings) == expected + ) @pytest.mark.parametrize( @@ -344,7 +348,7 @@ def test__get_option_string(strings, expected) -> None: parser = ArgumentParser() action = parser.add_argument(*strings) unparser = ReverseArgumentParser(parser, Namespace()) - assert unparser._get_option_string(action) == expected + assert unparser._get_option_string(action) == expected # noqa: S101 @pytest.mark.parametrize( @@ -360,7 +364,9 @@ def test__get_option_string_prefer_short(strings, expected) -> None: parser = ArgumentParser() action = parser.add_argument(*strings) unparser = ReverseArgumentParser(parser, Namespace()) - assert unparser._get_option_string(action, prefer_short=True) == expected + assert ( # noqa: S101 + unparser._get_option_string(action, prefer_short=True) == expected + ) @pytest.mark.parametrize( @@ -388,7 +394,7 @@ def test__unparse_store_action(add_args, add_kwargs, args, expected) -> None: namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_store_action(action) - assert unparser._args[1:] == [expected] + assert unparser._args[1:] == [expected] # noqa: S101 @pytest.mark.parametrize( @@ -424,7 +430,9 @@ def test__unparse_store_const_action( namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_store_const_action(action) - assert unparser._args[1:] == ([expected] if expected is not None else []) + assert ( # noqa: S101 + unparser._args[1:] == ([expected] if expected is not None else []) + ) @pytest.mark.parametrize( @@ -437,7 +445,9 @@ def test__unparse_store_true_action(args, expected) -> None: namespace = parser.parse_args(args) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_store_true_action(action) - assert unparser._args[1:] == ([expected] if expected is not None else []) + assert ( # noqa: S101 + unparser._args[1:] == ([expected] if expected is not None else []) + ) @pytest.mark.parametrize( @@ -450,7 +460,9 @@ def test__unparse_store_false_action(args, expected) -> None: namespace = parser.parse_args(args) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_store_false_action(action) - assert unparser._args[1:] == ([expected] if expected is not None else []) + assert ( # noqa: S101 + unparser._args[1:] == ([expected] if expected is not None else []) + ) @pytest.mark.parametrize( @@ -477,7 +489,7 @@ def test__unparse_append_action(add_args, add_kwargs, args, expected) -> None: namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_append_action(action) - assert unparser._args[1:] == expected + assert unparser._args[1:] == expected # noqa: S101 @pytest.mark.parametrize( @@ -492,7 +504,9 @@ def test__unparse_append_const_action(args, expected) -> None: namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_append_const_action(action) - assert unparser._args[1:] == ([expected] if expected is not None else []) + assert ( # noqa: S101 + unparser._args[1:] == ([expected] if expected is not None else []) + ) @pytest.mark.parametrize( @@ -525,7 +539,7 @@ def test__unparse_count_action(add_args, add_kwargs, args, expected) -> None: namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_count_action(action) - assert unparser._args[1:] == [expected] + assert unparser._args[1:] == [expected] # noqa: S101 @pytest.mark.parametrize( @@ -554,9 +568,9 @@ def test__unparse_sub_parsers_action(args, expected, pretty) -> None: result = strip_first_entry( unparser.get_effective_command_line_invocation() ) - assert result == expected + assert result == expected # noqa: S101 result = strip_first_line(unparser.get_pretty_command_line_invocation()) - assert result == pretty + assert result == pretty # noqa: S101 def test__unparse_sub_parsers_action_nested() -> None: @@ -596,9 +610,9 @@ def test__unparse_sub_parsers_action_nested() -> None: result = strip_first_entry( unparser.get_effective_command_line_invocation() ) - assert result == args + assert result == args # noqa: S101 result = strip_first_line(unparser.get_pretty_command_line_invocation()) - assert result == pretty + assert result == pretty # noqa: S101 def test__unparse_extend_action() -> None: @@ -609,7 +623,7 @@ def test__unparse_extend_action() -> None: unparser = ReverseArgumentParser(parser, namespace) expected = " --foo bar baz bif" unparser._unparse_extend_action(action) - assert unparser._args[1:] == [expected] + assert unparser._args[1:] == [expected] # noqa: S101 @pytest.mark.parametrize( @@ -636,6 +650,6 @@ def test__unparse_boolean_optional_action(default, args, expected) -> None: namespace = parser.parse_args(shlex.split(args)) unparser = ReverseArgumentParser(parser, namespace) unparser._unparse_boolean_optional_action(action) - assert unparser._args[1:] == ( + assert unparser._args[1:] == ( # noqa: S101 [expected] if expected is not None else [] ) From 9006adbeefdbd08237325d91fbdf7ecf952b386d Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 12:38:15 -0600 Subject: [PATCH 04/12] refactor: Don't override Python builtins --- doc/source/conf.py | 2 +- test/test_reverse_argparse.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/source/conf.py b/doc/source/conf.py index 126cb1b..912505a 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -13,7 +13,7 @@ # -- Project information ------------------------------------------------------ project = "reverse_argparse" -copyright = ( +copyright = ( # noqa: A001 "2023–2024, National Technology & Engineering Solutions of Sandia, LLC " "(NTESS)" ) diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index a036ec8..55828a8 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -98,18 +98,18 @@ def parser() -> ArgumentParser: ] -def strip_first_entry(input: str) -> str: +def strip_first_entry(input_string: str) -> str: """ Remove the first entry of a space-delimited string. Args: - input: A space-delimited string. + input_string: A space-delimited string. Returns: The input string with the first element (and first space) removed. """ - return " ".join(input.split()[1:]) + return " ".join(input_string.split()[1:]) def test_strip_first_entry() -> None: @@ -117,17 +117,17 @@ def test_strip_first_entry() -> None: assert strip_first_entry("foo bar baz") == "bar baz" # noqa: S101 -def strip_first_line(input: str) -> str: +def strip_first_line(input_string: str) -> str: """ Remove the first line of a multi-line string. Args: - input: A multi-line string. + input_string: A multi-line string. Returns: The input string with the first line removed. """ - return "\n".join(input.splitlines()[1:]) + return "\n".join(input_string.splitlines()[1:]) def test_strip_first_line() -> None: From 25ff30ff9c19378108db1cc31f367e0a98e189d9 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:06:52 -0600 Subject: [PATCH 05/12] test: Use UTC timezone with datetime Running `flake8-datetimez` via Ruff yielded the following findings: example/test_examples.py:88:25: DTZ005 `datetime.datetime.now()` called without a `tz` argument example/test_examples.py:89:25: DTZ007 Naive datetime constructed using `datetime.datetime.strptime()` without %z example/test_examples.py:125:13: DTZ005 `datetime.datetime.now()` called without a `tz` argument example/test_examples.py:126:25: DTZ007 Naive datetime constructed using `datetime.datetime.strptime()` without %z Found 4 errors. Specifying timezone info resolves the issues. --- example/test_examples.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/example/test_examples.py b/example/test_examples.py index fb5d557..48b4644 100755 --- a/example/test_examples.py +++ b/example/test_examples.py @@ -10,7 +10,7 @@ import re import shlex import subprocess # nosec B404 -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from pathlib import Path @@ -85,10 +85,10 @@ def test_post_processing() -> None: """.strip() in result.stdout ) - thirty_miutes_ago = datetime.now() - timedelta(minutes=30) + thirty_miutes_ago = datetime.now(tz=timezone.utc) - timedelta(minutes=30) time_from_example = datetime.strptime( shlex.split(result.stdout)[-1], "%Y-%m-%d %H:%M:%S.%f" - ) + ).astimezone(timezone.utc) assert ( # noqa: S101 thirty_miutes_ago - time_from_example < timedelta(seconds=1) ) @@ -122,11 +122,11 @@ def test_pretty_printing() -> None: in result.stdout ) assert re.search(r"--src /\S+/file\.txt", result.stdout) # noqa: S101 - today = datetime.now() + today = datetime.now(tz=timezone.utc) time_from_example = datetime.strptime( shlex.split(result.stdout.splitlines()[-1])[-1], "%Y-%m-%d %H:%M:%S.%f", - ) + ).astimezone(timezone.utc) assert today - time_from_example < timedelta(seconds=1) # noqa: S101 From 844c71ab25ce20ebb22b111524740b2c991f3f29 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:14:54 -0600 Subject: [PATCH 06/12] refactor: Assign exception messages to variables Running `flake8-errmsg` via Ruff yielded the following findings: reverse_argparse/reverse_argparse.py:143:17: EM102 Exception must not use an f-string literal, assign to variable first reverse_argparse/reverse_argparse.py:461:17: EM102 Exception must not use an f-string literal, assign to variable first Found 2 errors. This changes resolves the issues. --- reverse_argparse/reverse_argparse.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index 2106b2d..6dda0a8 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -139,10 +139,11 @@ def _unparse_action(self, action: Action) -> None: # noqa: C901 ): self._unparse_boolean_optional_action(action) else: # pragma: no cover - raise NotImplementedError( + message = ( f"{self.__class__.__name__} does not yet support the " f"unparsing of {action_type} objects." ) + raise NotImplementedError(message) def _arg_is_default_and_help_is_suppressed(self, action: Action) -> bool: """ @@ -457,10 +458,11 @@ def _unparse_sub_parsers_action(self, action: Action) -> None: if action.choices is None or not isinstance( action.choices, dict ): # pragma: no cover - raise RuntimeError( + message = ( "This subparser action is missing its dictionary of " f"choices: {action}" ) + raise RuntimeError(message) for subcommand, subparser in action.choices.items(): self._parsers.append(subparser) self._unparsed.append(False) From a7ae1f6813797f48f3889d6b362217c55a853079 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:18:59 -0600 Subject: [PATCH 07/12] refactor: Remove unnecessary shebang lines --- reverse_argparse/reverse_argparse.py | 1 - setup.py | 1 - test/test_reverse_argparse.py | 1 - 3 files changed, 3 deletions(-) diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index 6dda0a8..20d87d3 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 """ The ``reverse_argparse`` module. diff --git a/setup.py b/setup.py index 1e683a0..02d0e33 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 """ Setup file for the ``reverse_argparse`` package. diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index 55828a8..15d4acb 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 """The unit test suite for the ``reverse_argparse`` package.""" # © 2024 National Technology & Engineering Solutions of Sandia, LLC From 25d9be8d014f0159c4328594af84fc723fdad6f9 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:29:59 -0600 Subject: [PATCH 08/12] test: Use tuples for parametrize variables --- test/test_reverse_argparse.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index 15d4acb..afaca56 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -205,7 +205,7 @@ def test_get_command_line_invocation_strip_spaces() -> None: @pytest.mark.parametrize( - "add_args, add_kwargs, args, expected", + ("add_args", "add_kwargs", "args", "expected"), [ (["--foo"], {"action": "store"}, "--foo bar", [" --foo bar"]), ( @@ -305,7 +305,7 @@ def test__arg_is_default_and_help_is_suppressed() -> None: @pytest.mark.parametrize( - "strings, expected", + ("strings", "expected"), [ (["-v", "--verbose"], ["--verbose"]), (["--foo", "-f", "--foo-bar"], ["--foo", "--foo-bar"]), @@ -319,7 +319,7 @@ def test__get_long_option_strings(strings, expected) -> None: @pytest.mark.parametrize( - "strings, expected", + ("strings", "expected"), [ (["-v", "--verbose"], ["-v"]), (["--foo", "-f", "--foo-bar"], ["-f"]), @@ -335,7 +335,7 @@ def test__get_short_option_strings(strings, expected) -> None: @pytest.mark.parametrize( - "strings, expected", + ("strings", "expected"), [ (["-v", "--verbose"], "--verbose"), (["--foo", "-f", "--foo-bar"], "--foo"), @@ -351,7 +351,7 @@ def test__get_option_string(strings, expected) -> None: @pytest.mark.parametrize( - "strings, expected", + ("strings", "expected"), [ (["-v", "--verbose"], "-v"), (["-f", "--foo", "-b"], "-f"), @@ -369,7 +369,7 @@ def test__get_option_string_prefer_short(strings, expected) -> None: @pytest.mark.parametrize( - "add_args, add_kwargs, args, expected", + ("add_args", "add_kwargs", "args", "expected"), [ (["positional"], {}, "val", " val"), (["-f", "--foo"], {}, "-f bar", " --foo bar"), @@ -397,7 +397,7 @@ def test__unparse_store_action(add_args, add_kwargs, args, expected) -> None: @pytest.mark.parametrize( - "add_args, add_kwargs, args, expected", + ("add_args", "add_kwargs", "args", "expected"), [ (["--foo"], {"action": "store_const", "const": 42}, "", None), ( @@ -435,7 +435,7 @@ def test__unparse_store_const_action( @pytest.mark.parametrize( - "args, expected", [(shlex.split("--foo"), " --foo"), ([], None)] + ("args", "expected"), [(shlex.split("--foo"), " --foo"), ([], None)] ) def test__unparse_store_true_action(args, expected) -> None: """Ensure ``store_true`` actions are handled appropriately.""" @@ -450,7 +450,7 @@ def test__unparse_store_true_action(args, expected) -> None: @pytest.mark.parametrize( - "args, expected", [(shlex.split("--foo"), " --foo"), ([], None)] + ("args", "expected"), [(shlex.split("--foo"), " --foo"), ([], None)] ) def test__unparse_store_false_action(args, expected) -> None: """Ensure ``store_false`` actions are handled appropriately.""" @@ -465,7 +465,7 @@ def test__unparse_store_false_action(args, expected) -> None: @pytest.mark.parametrize( - "add_args, add_kwargs, args, expected", + ("add_args", "add_kwargs", "args", "expected"), [ ( ["--foo"], @@ -492,7 +492,7 @@ def test__unparse_append_action(add_args, add_kwargs, args, expected) -> None: @pytest.mark.parametrize( - "args, expected", [("--foo", " --foo"), ("", None)] + ("args", "expected"), [("--foo", " --foo"), ("", None)] ) def test__unparse_append_const_action(args, expected) -> None: """Ensure ``append_const`` actions are handled appropriately.""" @@ -509,7 +509,7 @@ def test__unparse_append_const_action(args, expected) -> None: @pytest.mark.parametrize( - "add_args, add_kwargs, args, expected", + ("add_args", "add_kwargs", "args", "expected"), [ ( ["--foo"], @@ -542,7 +542,7 @@ def test__unparse_count_action(add_args, add_kwargs, args, expected) -> None: @pytest.mark.parametrize( - "args, expected, pretty", + ("args", "expected", "pretty"), [ ("a 12", "a 12", " a \\\n 12"), ( @@ -626,7 +626,7 @@ def test__unparse_extend_action() -> None: @pytest.mark.parametrize( - "default, args, expected", + ("default", "args", "expected"), [ (None, "", None), (None, "--bool-opt", " --bool-opt"), From 9d412ddc83a6b0ad5ad3acfb5a520eb0df20086b Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 13:47:05 -0600 Subject: [PATCH 09/12] refactor: Ignore particular type error --- example/post_processing.py | 2 +- example/pretty_printing.py | 2 +- example/relative_references.py | 2 +- example/subparsers.py | 5 ++++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/example/post_processing.py b/example/post_processing.py index 3d6ce64..a12907f 100755 --- a/example/post_processing.py +++ b/example/post_processing.py @@ -19,7 +19,7 @@ parser.add_argument("--foo") parser.add_argument("--bar", default="spam") parser.add_argument("--baz", type=int, default=42) -parser.add_argument("--src", type=os.path.abspath) # type: ignore +parser.add_argument("--src", type=os.path.abspath) # type: ignore[arg-type] parser.add_argument("--before") # Parse the command line arguments. diff --git a/example/pretty_printing.py b/example/pretty_printing.py index 3082c52..412a91e 100755 --- a/example/pretty_printing.py +++ b/example/pretty_printing.py @@ -19,7 +19,7 @@ parser.add_argument("--foo") parser.add_argument("--bar", default="spam") parser.add_argument("--baz", type=int, default=42) -parser.add_argument("--src", type=os.path.abspath) # type: ignore +parser.add_argument("--src", type=os.path.abspath) # type: ignore[arg-type] parser.add_argument("--before") # Parse the command line arguments. diff --git a/example/relative_references.py b/example/relative_references.py index d00f1c8..b5e149b 100755 --- a/example/relative_references.py +++ b/example/relative_references.py @@ -17,7 +17,7 @@ parser.add_argument("--foo") parser.add_argument("--bar", default="spam") parser.add_argument("--baz", type=int, default=42) -parser.add_argument("--src", type=os.path.abspath) # type: ignore +parser.add_argument("--src", type=os.path.abspath) # type: ignore[arg-type] # Parse the command line arguments. args = parser.parse_args() diff --git a/example/subparsers.py b/example/subparsers.py index d811bc1..5ad12d7 100755 --- a/example/subparsers.py +++ b/example/subparsers.py @@ -21,7 +21,10 @@ foo_parser.add_argument("--two", default="spam") foo_parser.add_argument("--three", type=int, default=42) bar_parser = subparsers.add_parser("bar") -bar_parser.add_argument("--four", type=os.path.abspath) # type: ignore +bar_parser.add_argument( + "--four", + type=os.path.abspath, # type: ignore[arg-type] +) bar_parser.add_argument("--five") # Parse the command line arguments. From 862bc62bee7a61659cb69e668f4a1f1d1e9c0c1f Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 15:54:03 -0600 Subject: [PATCH 10/12] refactor: Address Pylint findings --- reverse_argparse/reverse_argparse.py | 34 +++++++++++++++++----------- test/test_reverse_argparse.py | 24 +++++++++++++------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index 20d87d3..2cc35f7 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -19,6 +19,10 @@ from typing import List, Sequence +BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION = 9 +SHORT_OPTION_LENGTH = 2 + + class ReverseArgumentParser: """ Argument parsing in reverse. @@ -90,7 +94,7 @@ def _unparse_args(self) -> None: self._unparse_action(action) self._unparsed[-1] = True - def _unparse_action(self, action: Action) -> None: # noqa: C901 + def _unparse_action(self, action: Action) -> None: # noqa: C901, PLR0912 """ Unparse a single action. @@ -134,7 +138,7 @@ def _unparse_action(self, action: Action) -> None: # noqa: C901 return elif ( action_type == "BooleanOptionalAction" - and sys.version_info.minor >= 9 + and sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION ): self._unparse_boolean_optional_action(action) else: # pragma: no cover @@ -213,7 +217,7 @@ def _get_long_option_strings( return [ option for option in option_strings - if len(option) > 2 + if len(option) > SHORT_OPTION_LENGTH and option[0] in self._parsers[-1].prefix_chars and option[1] in self._parsers[-1].prefix_chars ] @@ -235,7 +239,8 @@ def _get_short_option_strings( return [ option for option in option_strings - if len(option) == 2 and option[0] in self._parsers[-1].prefix_chars + if len(option) == SHORT_OPTION_LENGTH + and option[0] in self._parsers[-1].prefix_chars ] def _get_option_string( @@ -342,10 +347,10 @@ def _unparse_store_action(self, action: Action) -> None: values = [values] needs_quotes_regex = re.compile(r"(.*\s.*)") for value in values: - value = str(value) - if needs_quotes_regex.search(value): - value = needs_quotes_regex.sub(r"'\1'", value) - result.append(value) + string_value = str(value) + if needs_quotes_regex.search(string_value): + string_value = needs_quotes_regex.sub(r"'\1'", string_value) + result.append(string_value) self._append_list_of_args(result) def _unparse_store_const_action(self, action: Action) -> None: @@ -399,13 +404,13 @@ def _unparse_append_action(self, action: Action) -> None: for entry in values: tmp = [flag] for value in entry: - value = quote_arg_if_necessary(str(value)) - tmp.append(value) + quoted_value = quote_arg_if_necessary(str(value)) + tmp.append(quoted_value) result.append(tmp) else: for value in values: - value = quote_arg_if_necessary(str(value)) - result.append([flag, value]) + quoted_value = quote_arg_if_necessary(str(value)) + result.append([flag, quoted_value]) self._append_list_of_list_of_args(result) def _unparse_append_const_action(self, action: Action) -> None: @@ -429,7 +434,10 @@ def _unparse_count_action(self, action: Action) -> None: value = getattr(self._namespace, action.dest) count = value if action.default is None else (value - action.default) flag = self._get_option_string(action, prefer_short=True) - if len(flag) == 2 and flag[0] in self._parsers[-1].prefix_chars: + if ( + len(flag) == SHORT_OPTION_LENGTH + and flag[0] in self._parsers[-1].prefix_chars + ): self._append_arg(flag[0] + flag[1] * count) else: self._append_list_of_args([flag for _ in range(count)]) diff --git a/test/test_reverse_argparse.py b/test/test_reverse_argparse.py index afaca56..223c328 100644 --- a/test/test_reverse_argparse.py +++ b/test/test_reverse_argparse.py @@ -10,14 +10,18 @@ import sys from argparse import SUPPRESS, ArgumentParser, Namespace -if sys.version_info.minor >= 9: - from argparse import BooleanOptionalAction - import pytest from reverse_argparse import ReverseArgumentParser +BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION = 9 + + +if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION: + from argparse import BooleanOptionalAction + + @pytest.fixture() def parser() -> ArgumentParser: """ @@ -47,7 +51,7 @@ def parser() -> ArgumentParser: ) p.add_argument("--verbose", "-v", action="count", default=2) p.add_argument("--ext", action="extend", nargs="*") - if sys.version_info.minor >= 9: + if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION: p.add_argument( "--bool-opt", action=BooleanOptionalAction, default=False ) @@ -148,7 +152,11 @@ def test_get_effective_command_line_invocation(parser, args) -> None: "app-nargs2-val --const --app-const1 --app-const2 -vv --ext " "ext-val1 ext-val2 ext-val3 " ) - + ("--no-bool-opt " if sys.version_info.minor >= 9 else "") + + ( + "--no-bool-opt " + if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION + else "" + ) + "pos1-val1 pos1-val2 pos2-val" ) result = strip_first_entry( @@ -179,7 +187,7 @@ def test_get_pretty_command_line_invocation(parser, args) -> None: --app-const2 \\ -vv \\ --ext ext-val1 ext-val2 ext-val3 \\""" - if sys.version_info.minor >= 9: + if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION: expected += "\n --no-bool-opt \\" expected += """\n pos1-val1 pos1-val2 \\ pos2-val""" @@ -268,7 +276,7 @@ def test__unparse_args_boolean_optional_action() -> None: With a ``BooleanOptionalAction``, which became available in Python 3.9. """ - if sys.version_info.minor >= 9: + if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION: parser = ArgumentParser() parser.add_argument("--foo", action=BooleanOptionalAction) try: @@ -641,7 +649,7 @@ def test__unparse_extend_action() -> None: ) def test__unparse_boolean_optional_action(default, args, expected) -> None: """Ensure ``BooleanOptionalAction`` actions are handled appropriately.""" - if sys.version_info.minor >= 9: + if sys.version_info.minor >= BOOLEAN_OPTIONAL_ACTION_MINOR_VERSION: parser = ArgumentParser() action = parser.add_argument( "--bool-opt", action=BooleanOptionalAction, default=default From 076e19951c8fb2241c85d4da063d166292054f25 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 15:21:49 -0600 Subject: [PATCH 11/12] refactor: Address Ruff-specific lint findings --- doc/source/conf.py | 4 ++-- reverse_argparse/reverse_argparse.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/source/conf.py b/doc/source/conf.py index 912505a..01bbb40 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -14,8 +14,8 @@ project = "reverse_argparse" copyright = ( # noqa: A001 - "2023–2024, National Technology & Engineering Solutions of Sandia, LLC " - "(NTESS)" + "2023–2024, National Technology & Engineering Solutions " # noqa: RUF001 + "of Sandia, LLC (NTESS)" ) author = "Jason M. Gates" version = "1.0.6" diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index 2cc35f7..fc2a3ca 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -493,7 +493,7 @@ def _unparse_extend_action(self, action: Action) -> None: values = getattr(self._namespace, action.dest) if values is not None: self._append_list_of_args( - [self._get_option_string(action)] + values + [self._get_option_string(action), *values] ) def _unparse_boolean_optional_action(self, action: Action) -> None: From f02c7f2eee4861904d3505fd7a6d2cc0fbc5cfc8 Mon Sep 17 00:00:00 2001 From: "Jason M. Gates" Date: Mon, 22 Apr 2024 12:30:03 -0600 Subject: [PATCH 12/12] patch: Force `prefer_short` to be keyword-only Turning on `flake8-boolean-trap` linting via Ruff resulted in the following findings: reverse_argparse/reverse_argparse.py:242:31: FBT002 Boolean default positional argument in function definition reverse_argparse/reverse_argparse.py:242:31: FBT001 Boolean-typed positional argument in function definition Found 2 errors. Switching `prefer_short` from a positional to a keyword-only argument addresses the problem. Note that this is technically a breaking change, but only for a "private" method, not in the package's public API. Therefore the change is not registered as a breaking change via Conventional Commit syntax, and no major version update will be created. Instead, this commit will force the creation of a patch release. If users were relying on the prior behavior of this internal method, they can simply switch to the keyword syntax when calling it. --- reverse_argparse/reverse_argparse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reverse_argparse/reverse_argparse.py b/reverse_argparse/reverse_argparse.py index fc2a3ca..8974253 100644 --- a/reverse_argparse/reverse_argparse.py +++ b/reverse_argparse/reverse_argparse.py @@ -244,7 +244,7 @@ def _get_short_option_strings( ] def _get_option_string( - self, action: Action, prefer_short: bool = False + self, action: Action, *, prefer_short: bool = False ) -> str: """ Get the option string for the `action`.