507 usethis docstyle uses ini conventions in toml file (#510)

* Add failing test

* Add passing test

* Create workaround for tomlkit issue

* Tweak test

* Avoid setting pyprojec.toml config as dict in tests

* Have multiple behaviours to preserve style as much as possible

* Handle edge case (bug?) in tomlkit's handling of key deletion from a table

* Add test and simplify deletion logic

Author: nathanjmcdougall

.pre-commit-config.yaml

@@ -8,14 +8,14 @@ repos:
     hooks:
       - id: uv-lock
         name: uv-lock
-        entry: uv lock
+        entry: uv lock --offline
         files: ^(uv\.lock|pyproject\.toml|uv\.toml)$
         language: system
         always_run: true
         pass_filenames: false       
       - id: uv-sync
         name: uv-sync
-        entry: uv sync --no-active
+        entry: uv sync --no-active --offline
         args: ["--locked"]
         language: system
         always_run: true

src/usethis/_integrations/file/toml/io_.py

@@ -1,10 +1,12 @@
 from __future__ import annotations
 
+import contextlib
 import copy
 from typing import TYPE_CHECKING, Any
 
 import mergedeep
 import tomlkit.api
+import tomlkit.items
 from pydantic import TypeAdapter
 from tomlkit import TOMLDocument
 from tomlkit.exceptions import TOMLKitError
@@ -121,50 +123,62 @@ def set_value(
         """
         toml_document = copy.copy(self.get())
 
+        if not keys:
+            # Root level config - value must be a mapping.
+            TypeAdapter(dict).validate_python(toml_document)
+            assert isinstance(toml_document, dict)
+            TypeAdapter(dict).validate_python(value)
+            assert isinstance(value, dict)
+            if not toml_document or exists_ok:
+                toml_document.update(value)
+                self.commit(toml_document)
+                return
+
+        d, parent = toml_document, {}
+        shared_keys = []
         try:
             # Index our way into each ID key.
             # Eventually, we should land at a final dict, which is the one we are setting.
-            d, parent = toml_document, {}
-            if not keys:
-                # Root level config - value must be a mapping.
-                TypeAdapter(dict).validate_python(d)
-                assert isinstance(d, dict)
-                TypeAdapter(dict).validate_python(value)
-                assert isinstance(value, dict)
-                if not d:
-                    raise KeyError
             for key in keys:
                 TypeAdapter(dict).validate_python(d)
                 assert isinstance(d, dict)
                 d, parent = d[key], d
+                shared_keys.append(key)
         except KeyError:
             # The old configuration should be kept for all ID keys except the
             # final/deepest one which shouldn't exist anyway since we checked as much,
             # above. For example, if there is [tool.ruff] then we shouldn't overwrite it
             # with [tool.deptry]; they should coexist. So under the "tool" key, we need
-            # to merge the two dicts.
-            contents = value
-            for key in reversed(keys):
-                contents = {key: contents}
-            toml_document = mergedeep.merge(toml_document, contents)  # type: ignore[reportAssignmentType]
-            assert isinstance(toml_document, TOMLDocument)
+            # to "merge" the two dicts.
+
+            if len(keys) <= 3:
+                contents = value
+                for key in reversed(keys):
+                    contents = {key: contents}
+                toml_document = mergedeep.merge(toml_document, contents)  # type: ignore[reportAssignmentType]
+                assert isinstance(toml_document, TOMLDocument)
+            else:
+                # Note that this alternative logic is just to avoid a bug:
+                # https://github.com/nathanjmcdougall/usethis-python/issues/507
+                TypeAdapter(dict).validate_python(d)
+                assert isinstance(d, dict)
+
+                unshared_keys = keys[len(shared_keys) :]
+
+                d[_get_unified_key(unshared_keys)] = value
         else:
             if not exists_ok:
                 # The configuration is already present, which is not allowed.
                 if keys:
                     msg = f"Configuration value '{'.'.join(keys)}' is already set."
                 else:
-                    msg = "Configuration value is at root level is already set."
+                    msg = "Configuration value at root level is already set."
                 raise TOMLValueAlreadySetError(msg)
             else:
                 # The configuration is already present, but we're allowed to overwrite it.
                 TypeAdapter(dict).validate_python(parent)
                 assert isinstance(parent, dict)
-                if parent:
-                    parent[keys[-1]] = value
-                else:
-                    # i.e. the case where we're creating the root of the document
-                    toml_document.update(value)
+                parent[keys[-1]] = value
 
         self.commit(toml_document)
 
@@ -204,7 +218,14 @@ def __delitem__(self, keys: list[str]) -> None:
             for key in list(d.keys()):
                 del d[key]
         else:
-            del d[keys[-1]]
+            with contextlib.suppress(KeyError):
+                # There is a strange behaviour (bug?) in tomlkit where deleting a key
+                # has two separate lines:
+                # self._value.remove(key)  # noqa: ERA001
+                # dict.__delitem__(self, key)  # noqa: ERA001
+                # but it's not clear why there's this duplicate and it causes a KeyError
+                # in some cases.
+                d.remove(keys[-1])
 
             # Cleanup: any empty sections should be removed.
             for idx in range(len(keys) - 1):
@@ -286,3 +307,12 @@ def remove_from_list(self, *, keys: list[str], values: list[Any]) -> None:
         p_parent[keys[-1]] = new_values
 
         self.commit(toml_document)
+
+
+def _get_unified_key(keys: list[str]) -> tomlkit.items.Key:
+    single_keys = [tomlkit.items.SingleKey(key) for key in keys]
+    if len(single_keys) == 1:
+        (unified_key,) = single_keys
+    else:
+        unified_key = tomlkit.items.DottedKey(single_keys)
+    return unified_key

src/usethis/_integrations/uv/init.py

@@ -36,6 +36,6 @@ def ensure_pyproject_toml(*, author: bool = True) -> None:
     if not ((Path.cwd() / "src").exists() and (Path.cwd() / "src").is_dir()):
         # hatch needs to know where to find the package
         PyprojectTOMLManager().set_value(
-            keys=["tool", "hatch", "build", "targets", "wheel"],
-            value={"packages": ["."]},
+            keys=["tool", "hatch", "build", "targets", "wheel", "packages"],
+            value=["."],
         )

tests/usethis/_integrations/file/toml/test_toml_io_.py

@@ -1,8 +1,14 @@
 from pathlib import Path
 
 import pytest
-
-from usethis._integrations.file.toml.errors import TOMLValueAlreadySetError
+import tomlkit
+import tomlkit.api
+import tomlkit.items
+
+from usethis._integrations.file.toml.errors import (
+    TOMLValueAlreadySetError,
+    TOMLValueMissingError,
+)
 from usethis._integrations.file.toml.io_ import TOMLFileManager
 from usethis._test import change_cwd
 
@@ -71,6 +77,131 @@ def relative_path(self) -> Path:
             # Assert
             assert manager._content == 42
 
+        def test_set_high_levels_of_nesting(self, tmp_path: Path) -> None:
+            # Arrange
+            class MyTOMLFileManager(TOMLFileManager):
+                @property
+                def relative_path(self) -> Path:
+                    return Path("pyproject.toml")
+
+            with change_cwd(tmp_path), MyTOMLFileManager() as manager:
+                (tmp_path / "pyproject.toml").touch()
+
+                # Act
+                manager.set_value(
+                    keys=[],
+                    value={
+                        "project": {"name": "usethis", "version": "0.1.0"},
+                        "tool": {
+                            "ruff": {
+                                "lint": {
+                                    "select": ["A"],
+                                    "pydocstyle": {"convention": "pep257"},
+                                }
+                            }
+                        },
+                    },
+                )
+
+            # Assert
+            contents = (tmp_path / "pyproject.toml").read_text()
+            assert contents == (
+                """\
+[project]
+name = "usethis"
+version = "0.1.0"
+
+[tool.ruff.lint]
+select = ["A"]
+
+[tool.ruff.lint.pydocstyle]
+convention = "pep257"
+"""
+            )
+
+        def test_set_high_levels_of_nesting_in_existing(self, tmp_path: Path) -> None:
+            # https://github.com/nathanjmcdougall/usethis-python/issues/507
+            # Arrange
+            class MyTOMLFileManager(TOMLFileManager):
+                @property
+                def relative_path(self) -> Path:
+                    return Path("pyproject.toml")
+
+            with change_cwd(tmp_path), MyTOMLFileManager() as manager:
+                pyproject_toml_path = tmp_path / "pyproject.toml"
+                pyproject_toml_path.write_text(
+                    """\
+[project]
+name = "usethis"
+version = "0.1.0"
+
+[tool.ruff]
+lint.select = [ "A" ]
+"""
+                )
+
+                # Act
+                manager.set_value(
+                    keys=["tool", "ruff", "lint", "pydocstyle", "convention"],
+                    value="pep257",
+                )
+
+            # Assert
+            contents = (tmp_path / "pyproject.toml").read_text()
+            assert contents == (
+                """\
+[project]
+name = "usethis"
+version = "0.1.0"
+
+[tool.ruff]
+lint.select = [ "A" ]
+lint.pydocstyle.convention = "pep257"
+"""
+            )
+
+        def test_tomlkit_high_levels_of_nesting_in_existing(
+            self, tmp_path: Path
+        ) -> None:
+            # To help debug https://github.com/nathanjmcdougall/usethis-python/issues/507
+            # This proves that dottedkey works.
+
+            # Arrange
+            txt = """\
+[tool.ruff]
+lint.select = [ "A" ]
+"""
+
+            toml_document: tomlkit.TOMLDocument = tomlkit.api.parse(txt)
+
+            # Act
+            tool_section = toml_document["tool"]
+            assert isinstance(tool_section, tomlkit.items.Table)
+            ruff_section = tool_section["ruff"]
+            assert isinstance(ruff_section, tomlkit.items.Table)
+            lint_section = ruff_section["lint"]
+            assert isinstance(lint_section, tomlkit.items.Table)
+            lint_section[
+                tomlkit.items.DottedKey(
+                    [
+                        tomlkit.items.SingleKey("pydocstyle"),
+                        tomlkit.items.SingleKey("convention"),
+                    ]
+                )
+            ] = "pep257"
+            # Whereas this doesn't work:
+            # lint_section["pydocstyle"] = {"convention": "pep257"}  # noqa: ERA001
+
+            # Assert
+            contents = tomlkit.api.dumps(toml_document)
+            assert contents == (
+                """\
+[tool.ruff]
+lint.select = [ "A" ]
+lint.pydocstyle.convention = "pep257"
+"""
+            )
+
     class TestSetValue:
         def test_no_keys(self, tmp_path: Path) -> None:
             # Arrange
@@ -123,7 +254,7 @@ def relative_path(self) -> Path:
                 # Act, Assert
                 with pytest.raises(
                     TOMLValueAlreadySetError,
-                    match="Configuration value is at root level is already set.",
+                    match="Configuration value at root level is already set.",
                 ):
                     manager.set_value(keys=[], value={"a": "c"}, exists_ok=False)
 
@@ -180,7 +311,7 @@ def relative_path(self) -> Path:
                 ):
                     manager.set_value(keys=["a"], value=["d"])
 
-    class TestDel:
+    class TestDelItem:
         def test_no_keys(self, tmp_path: Path) -> None:
             # Arrange
             class MyTOMLFileManager(TOMLFileManager):
@@ -234,6 +365,22 @@ def relative_path(self) -> Path:
                 # Assert
                 assert not manager._content
 
+        def test_removing_doesnt_exist(self, tmp_path: Path) -> None:
+            # Arrange
+            class MyTOMLFileManager(TOMLFileManager):
+                @property
+                def relative_path(self) -> Path:
+                    return Path("pyproject.toml")
+
+            with change_cwd(tmp_path), MyTOMLFileManager() as manager:
+                (tmp_path / "pyproject.toml").touch()
+
+                manager[["a"]] = "b"
+
+                # Act
+                with pytest.raises(TOMLValueMissingError):
+                    del manager[["c"]]
+
     class TestExtendList:
         def test_inplace_modifications(self, tmp_path: Path) -> None:
             # Arrange

tests/usethis/_interface/test_docstyle.py

@@ -36,3 +36,56 @@ def test_invalid_pyproject_toml(
 
         # Assert
         assert result.exit_code == 1, result.output
+
+    def test_pyproject_toml_success(self, tmp_path: Path):
+        # https://github.com/nathanjmcdougall/usethis-python/issues/507
+
+        # Arrange
+        valid_pyproject_toml = tmp_path / "pyproject.toml"
+        valid_pyproject_toml.touch()
+
+        # Act
+        with change_cwd(tmp_path):
+            runner = CliRunner()
+            result = runner.invoke(app, ["docstyle", "numpy"])
+
+        # Assert
+        assert result.exit_code == 0, result.output
+        assert (
+            "✔ Setting docstring style to 'numpy' in 'pyproject.toml'." in result.output
+        )
+
+        assert valid_pyproject_toml.exists()
+        content = valid_pyproject_toml.read_text()
+        assert (
+            """\
+[tool.ruff]
+line-length = 88
+lint.pydocstyle.convention = "numpy"\
+"""
+            in content
+        )
+
+    def test_adding_to_existing_file(self, tmp_path: Path):
+        # Arrange
+        existing_pyproject_toml = tmp_path / "pyproject.toml"
+        existing_pyproject_toml.write_text(
+            """\
+[project]
+name = "usethis"
+version = "0.1.0"
+
+[tool.ruff]
+lint.select = [ "A" ]
+"""
+        )
+
+        # Act
+        with change_cwd(tmp_path):
+            runner = CliRunner()
+            result = runner.invoke(app, ["docstyle", "pep257"])
+
+        # Assert
+        assert result.exit_code == 0, result.output
+        content = existing_pyproject_toml.read_text()
+        assert "[lint.pydocstyle]" not in content  # Wrong section name