Enable pydocstyle checks in ruff

Some of them: many are left disabled, and will remain disabled, and some
of the disabled will get enabled ;) The resulting patch would be too
large, so far I enabled mostly checks that either pass or break already
accepted rules (e.g. "a single-line docstring shall fit on a single
line").

docs/conf.py

@@ -306,9 +306,8 @@ def _load_theme(
 
 
 def generate_tmt_docs(app: Sphinx) -> None:
-    """
-    Run `make generate` to populate the auto-generated documentations
-    """
+    """ Run `make generate` to populate the auto-generated sources """
+
     conf_dir = Path(app.confdir)
     subprocess.run(["make", "generate"], cwd=conf_dir)
 

pyproject.toml

@@ -288,6 +288,7 @@ lint.select = [
     "W",  # pycodestyle
     "I",  # isort
     "N",  # pep8-naming
+    "D",    # pydocstyle
     "UP",  # pyupgrade
     "B",  # flake8-bugbear
     "C4",  # flake8-comprehensions
@@ -321,11 +322,42 @@ lint.ignore = [
     "PLE1205",  # Too many arguments for `logging` format string
     "RUF012",  # Mutable class attributes should be annotated with `typing.ClassVar`
     "RUF013",  # PEP 484 prohibits implicit `Optional`
+
+    # pydocstyle
+    # TODO: some of these will be enabled in their own patches
+    "D100",  # Missing docstring in public module
+    "D101",  # Missing docstring in public class
+    "D102",  # Missing docstring in public method
+    "D103",  # Missing docstring in public function
+    "D104",  # Missing docstring in public package
+    "D105",  # Missing docstring in magic method
+    "D106",  # Missing docstring in public nested class
+    "D107",  # Missing docstring in __init__
+    "D202",  # No blank lines allowed after function docstring
+    "D203",  # 1 blank line required before class docstring
+    "D204",  # 1 blank line required after class docstring
+    "D205",  # 1 blank line required between summary line and description
+    "D210",  # No whitespaces allowed surrounding docstring text
+    "D212",  # Multi-line docstring summary should start at the first line
+    "D301",  # Use r""" if any backslashes in a docstring
+    "D400",  # First line should end with a period
+    "D401",  # First line of docstring should be in imperative mood
+    "D415",  # First line should end with a period, question mark, or exclamation point
     ]
 
 [tool.ruff.lint.flake8-bugbear]
 extend-immutable-calls = ["tmt.utils.field"]
 
+[tool.ruff.lint.pydocstyle]
+# "The PEP 257 convention includes all D errors apart from: D203, D212,
+# D213, D214, D215, D404, D405, D406, D407, D408, D409, D410, D411, D413,
+# D415, D416, and D417."
+#
+# See https://docs.astral.sh/ruff/faq/#does-ruff-support-numpy-or-google-style-docstrings for
+# the most up-to-date info.
+convention = "pep257"
+property-decorators = ["tmt.utils.cached_property"]
+
 [tool.ruff.lint.isort]
 known-first-party = ["tmt"]
 

tests/unit/test_base.py

@@ -176,9 +176,7 @@ def test_pickleable_tree() -> None:
 
 
 def test_expand_node_data(monkeypatch) -> None:
-    """
-    Test :py:func:`tmt.base.expand_node_data` handles various forms of variables.
-    """
+    """ :py:func:`tmt.base.expand_node_data` handles various forms of variables """
 
     # From ``_data` and `_expected` we construct lists with items, including
     # another list and a dictionary, to verify `expand_node_data()` handles

tests/unit/test_package_managers.py

@@ -80,9 +80,7 @@ def has_legacy_dnf(container: ContainerData) -> bool:
 
 
 def has_dnf5_preinstalled(container: ContainerData) -> bool:
-    """
-    Checks whether a container provides ``dnf5``.
-    """
+    """ Checks whether a container provides ``dnf5`` """
 
     return container.image_url_or_id in (
         CONTAINER_FEDORA_RAWHIDE.url,

tests/unit/test_utils.py

@@ -931,9 +931,7 @@ def test_in_repository(
 # tmt.utils.wait() & waiting for things to happen
 #
 def test_wait_bad_tick(root_logger):
-    """
-    :py:func:`wait` shall raise an exception when invalid ``tick`` is given.
-    """
+    """ :py:func:`wait` shall raise an exception when invalid ``tick`` is given """
 
     with pytest.raises(GeneralError, match='Tick must be a positive integer'):
         wait(Common(logger=root_logger), lambda: False, datetime.timedelta(seconds=1), tick=-1)

tmt/cli.py

@@ -1994,9 +1994,7 @@ def lint(
 
 @main.group(cls=CustomGroup)
 def setup(**kwargs: Any) -> None:
-    """
-    Setup the environment for working with tmt.
-    """
+    """ Setup the environment for working with tmt """
 
 
 @setup.group(cls=CustomGroup)
@@ -2050,9 +2048,7 @@ def setup_completion(shell: str, install: bool) -> None:
          '~/.bashrc'.
          """)
 def completion_bash(context: Context, install: bool, **kwargs: Any) -> None:
-    """
-    Setup shell completions for bash.
-    """
+    """ Setup shell completions for bash """
     setup_completion('bash', install)
 
 
@@ -2065,9 +2061,7 @@ def completion_bash(context: Context, install: bool, **kwargs: Any) -> None:
          '~/.zshrc'.
          """)
 def completion_zsh(context: Context, install: bool, **kwargs: Any) -> None:
-    """
-    Setup shell completions for zsh.
-    """
+    """ Setup shell completions for zsh """
     setup_completion('zsh', install)
 
 
@@ -2077,7 +2071,5 @@ def completion_zsh(context: Context, install: bool, **kwargs: Any) -> None:
     '--install', '-i', 'install', is_flag=True,
     help="Persistently store the script to '~/.config/fish/completions/tmt.fish'.")
 def completion_fish(context: Context, install: bool, **kwargs: Any) -> None:
-    """
-    Setup shell completions for fish.
-    """
+    """ Setup shell completions for fish """
     setup_completion('fish', install)

tmt/convert.py

@@ -56,9 +56,7 @@ def read_manual(
         disabled: bool,
         with_script: bool,
         logger: tmt.log.Logger) -> None:
-    """
-    Reads metadata of manual test cases from Nitrate
-    """
+    """ Reads metadata of manual test cases from Nitrate """
     import tmt.export.nitrate
     nitrate = tmt.export.nitrate.import_nitrate()
     # Turns off nitrate caching

tmt/hardware.py

@@ -81,9 +81,7 @@
 
 
 class Operator(enum.Enum):
-    """
-    Binary operators defined by specification.
-    """
+    """ Binary operators defined by specification """
 
     EQ = '=='
     NEQ = '!='
@@ -184,9 +182,7 @@ class Operator(enum.Enum):
 
 
 class ConstraintNameComponents(NamedTuple):
-    """
-    Components of a constraint name.
-    """
+    """ Components of a constraint name """
 
     #: ``disk`` of ``disk[1].size``
     name: str
@@ -198,9 +194,7 @@ class ConstraintNameComponents(NamedTuple):
 
 @dataclasses.dataclass
 class ConstraintComponents:
-    """
-    Components of a constraint.
-    """
+    """ Components of a constraint """
 
     name: str
     peer_index: Optional[int]
@@ -303,9 +297,7 @@ def not_contains(haystack: list[str], needle: str) -> bool:
 
 
 class ParseError(tmt.utils.MetadataError):
-    """
-    Raised when HW requirement parsing fails.
-    """
+    """ Raised when HW requirement parsing fails """
 
     def __init__(self, constraint_name: str, raw_value: str,
                  message: Optional[str] = None) -> None:
@@ -329,9 +321,7 @@ def __init__(self, constraint_name: str, raw_value: str,
 
 @dataclasses.dataclass(repr=False)
 class BaseConstraint(SpecBasedContainer[Spec, Spec]):
-    """
-    Base class for all classes representing one or more constraints.
-    """
+    """ Base class for all classes representing one or more constraints """
 
     @classmethod
     def from_spec(cls, spec: Any) -> 'BaseConstraint':
@@ -394,9 +384,7 @@ def variant(self) -> list['Constraint[Any]']:
 
 @dataclasses.dataclass(repr=False)
 class CompoundConstraint(BaseConstraint):
-    """
-    Base class for all *compound* constraints.
-    """
+    """ Base class for all *compound* constraints """
 
     def __init__(
             self,
@@ -467,9 +455,7 @@ def variants(
 
 @dataclasses.dataclass(repr=False)
 class Constraint(BaseConstraint, Generic[ConstraintValueT]):
-    """
-    A constraint imposing a particular limit to one of the guest properties.
-    """
+    """ A constraint imposing a particular limit to one of the guest properties """
 
     # Name of the constraint. Used for logging purposes, usually matches the
     # name of the system property.
@@ -744,9 +730,7 @@ def from_specification(
 
 @dataclasses.dataclass(repr=False)
 class And(CompoundConstraint):
-    """
-    Represents constraints that are grouped in ``and`` fashion.
-    """
+    """ Represents constraints that are grouped in ``and`` fashion """
 
     def __init__(self, constraints: Optional[list[BaseConstraint]] = None) -> None:
         """
@@ -802,9 +786,7 @@ def variants(
 
 @dataclasses.dataclass(repr=False)
 class Or(CompoundConstraint):
-    """
-    Represents constraints that are grouped in ``or`` fashion.
-    """
+    """ Represents constraints that are grouped in ``or`` fashion """
 
     def __init__(self, constraints: Optional[list[BaseConstraint]] = None) -> None:
         """

tmt/steps/__init__.py

@@ -2084,9 +2084,7 @@ def push(
             guest: 'Guest',
             filename_base: Optional[Path] = None,
             logger: tmt.log.Logger) -> Environment:
-        """
-        Save and push topology to a given guest.
-        """
+        """ Save and push topology to a given guest """
 
         topology_filepaths = self.save(dirpath=dirpath, filename_base=filename_base)
 

tmt/steps/discover/__init__.py

@@ -141,9 +141,7 @@ def download_distgit_source(
             )
 
     def log_import_plan_details(self) -> None:
-        """
-        Log details about the imported plan
-        """
+        """ Log details about the imported plan """
         parent = cast(Optional[tmt.steps.discover.Discover], self.parent)
         if parent and parent.plan._original_plan and \
                 parent.plan._original_plan._remote_plan_fmf_id:
@@ -155,9 +153,7 @@ def log_import_plan_details(self) -> None:
                 self.verbose(f'import {key}', value, 'green')
 
     def post_dist_git(self, created_content: list[Path]) -> None:
-        """
-        Discover tests after dist-git applied patches
-        """
+        """ Discover tests after dist-git applied patches """
         pass
 
 

tmt/steps/execute/__init__.py

@@ -527,9 +527,7 @@ def prepare_tests(self, guest: Guest, logger: tmt.log.Logger) -> list[TestInvoca
         return invocations
 
     def prepare_scripts(self, guest: "tmt.steps.provision.Guest") -> None:
-        """
-        Prepare additional scripts for testing
-        """
+        """ Prepare additional scripts for testing """
         # Install all scripts on guest
         for script in self.scripts:
             source = SCRIPTS_SRC_DIR / script.path.name
@@ -590,9 +588,7 @@ def load_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Resul
             note=note)]
 
     def load_custom_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
-        """
-        Process custom results.yaml file created by the test itself.
-        """
+        """ Process custom results.yaml file created by the test itself """
         test, guest = invocation.test, invocation.guest
 
         custom_results_path_yaml = invocation.test_data_path / 'results.yaml'
@@ -783,9 +779,7 @@ def run_checks_after_test(
 
 
 class Execute(tmt.steps.Step):
-    """
-    Run tests using the specified executor.
-    """
+    """ Run tests using the specified executor """
 
     # Internal executor is the default implementation
     DEFAULT_HOW = 'tmt'

tmt/steps/provision/__init__.py

@@ -406,6 +406,8 @@ def _query_package_manager(
 
     def _query_has_selinux(self, guest: 'Guest') -> Optional[bool]:
         """
+        Detect whether guest uses SELinux.
+
         For detection ``/proc/filesystems`` is used, see ``man 5 filesystems`` for details.
         """
 
@@ -1090,9 +1092,7 @@ def push(self,
              destination: Optional[Path] = None,
              options: Optional[list[str]] = None,
              superuser: bool = False) -> None:
-        """
-        Push files to the guest
-        """
+        """ Push files to the guest """
 
         raise NotImplementedError
 
@@ -1101,9 +1101,7 @@ def pull(self,
              destination: Optional[Path] = None,
              options: Optional[list[str]] = None,
              extend_options: Optional[list[str]] = None) -> None:
-        """
-        Pull files from the guest
-        """
+        """ Pull files from the guest """
 
         raise NotImplementedError
 

tmt/steps/report/polarion.py

@@ -183,9 +183,7 @@ class ReportPolarionData(tmt.steps.report.ReportStepData):
 
 @tmt.steps.provides_method('polarion')
 class ReportPolarion(tmt.steps.report.ReportPlugin[ReportPolarionData]):
-    """
-    Write test results into an xUnit file and upload to Polarion.
-    """
+    """ Write test results into an xUnit file and upload to Polarion """
 
     _data_class = ReportPolarionData
 

tmt/steps/report/reportportal.py

@@ -224,9 +224,7 @@ class ReportReportPortal(tmt.steps.report.ReportPlugin[ReportReportPortalData]):
         }
 
     def handle_response(self, response: requests.Response) -> None:
-        """
-        Check the endpoint response and raise an exception if needed.
-        """
+        """ Check the endpoint response and raise an exception if needed """
 
         if not response.ok:
             raise tmt.utils.ReportError(
@@ -236,9 +234,7 @@ def handle_response(self, response: requests.Response) -> None:
         self.debug("Message from the endpoint", response.text)
 
     def check_options(self) -> None:
-        """
-        Write warning if there might be caused an unexpected behaviour by the option combinations
-        """
+        """ Check options for known troublesome combinations """
         # TODO: Update restriction of forbidden option combinations based on feedback.
 
         if self.data.launch_per_plan and self.data.suite_per_plan: