reviews part 2

Author: RonnyPfannschmidt

src/setuptools_scm/config.py

@@ -6,6 +6,7 @@
 import warnings
 from typing import Any
 from typing import Callable
+from typing import cast
 from typing import Pattern
 from typing import TYPE_CHECKING
 from typing import Union
@@ -70,7 +71,36 @@ def _lazy_tomli_load(data: str) -> dict[str, Any]:
     return loads(data)
 
 
-VersionT = Union[Version, NonNormalizedVersion]
+_VersionT = Union[Version, NonNormalizedVersion]
+
+
+def _validate_version_cls(
+    version_cls: type[_VersionT] | str | None, normalize: bool
+) -> type[_VersionT]:
+    if not normalize:
+        # `normalize = False` means `version_cls = NonNormalizedVersion`
+        if version_cls is not None:
+            raise ValueError(
+                "Providing a custom `version_cls` is not permitted when "
+                "`normalize=False`"
+            )
+        return NonNormalizedVersion
+    else:
+        # Use `version_cls` if provided, default to packaging or pkg_resources
+        if version_cls is None:
+            return Version
+        elif isinstance(version_cls, str):
+            try:
+                # Not sure this will work in old python
+                import importlib
+
+                pkg, cls_name = version_cls.rsplit(".", 1)
+                version_cls_host = importlib.import_module(pkg)
+                return cast(type[_VersionT], getattr(version_cls_host, cls_name))
+            except:  # noqa
+                raise ValueError(f"Unable to import version_cls='{version_cls}'")
+        else:
+            return version_cls
 
 
 class Configuration:
@@ -79,7 +109,7 @@ class Configuration:
     parent: _t.PathT | None
     _root: str
     _relative_to: str | None
-    version_cls: type[VersionT]
+    version_cls: type[_VersionT]
 
     def __init__(
         self,
@@ -98,11 +128,7 @@ def __init__(
         parse: Any | None = None,
         git_describe_command: _t.CMD_TYPE | None = None,
         dist_name: str | None = None,
-        version_cls: type[Version]
-        | type[NonNormalizedVersion]
-        | type
-        | str
-        | None = None,
+        version_cls: type[_VersionT] | type | str | None = None,
         normalize: bool = True,
         search_parent_directories: bool = False,
     ):
@@ -125,30 +151,7 @@ def __init__(
         self.search_parent_directories = search_parent_directories
         self.parent = None
 
-        if not normalize:
-            # `normalize = False` means `version_cls = NonNormalizedVersion`
-            if version_cls is not None:
-                raise ValueError(
-                    "Providing a custom `version_cls` is not permitted when "
-                    "`normalize=False`"
-                )
-            self.version_cls = NonNormalizedVersion
-        else:
-            # Use `version_cls` if provided, default to packaging or pkg_resources
-            if version_cls is None:
-                self.version_cls = Version
-            elif isinstance(version_cls, str):
-                try:
-                    # Not sure this will work in old python
-                    import importlib
-
-                    pkg, cls_name = version_cls.rsplit(".", 1)
-                    version_cls_host = importlib.import_module(pkg)
-                    self.version_cls = getattr(version_cls_host, cls_name)
-                except:  # noqa
-                    raise ValueError(f"Unable to import version_cls='{version_cls}'")
-            else:
-                self.version_cls = version_cls
+        self.version_cls = _validate_version_cls(version_cls, normalize)
 
     @property
     def fallback_root(self) -> str:

src/setuptools_scm/utils.py

@@ -3,13 +3,14 @@
 """
 from __future__ import annotations
 
-import inspect
 import os
 import platform
 import shlex
 import subprocess
 import sys
 import warnings
+from types import CodeType
+from types import FunctionType
 from typing import Iterator
 from typing import Mapping
 
@@ -52,11 +53,10 @@ def ensure_stripped_str(str_or_bytes: str | bytes) -> str:
         return str_or_bytes.decode("utf-8", "surrogateescape").strip()
 
 
-def _popen_pipes(cmd: _t.CMD_TYPE, cwd: _t.PathT) -> subprocess.Popen[bytes]:
-    return subprocess.Popen(
+def _run(cmd: _t.CMD_TYPE, cwd: _t.PathT) -> subprocess.CompletedProcess[bytes]:
+    return subprocess.run(
         cmd,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
+        capture_output=True,
         cwd=str(cwd),
         env=dict(
             no_git_env(os.environ),
@@ -75,16 +75,15 @@ def do_ex(cmd: _t.CMD_TYPE, cwd: _t.PathT = ".") -> _t.CmdResult:
     if os.name == "posix" and not isinstance(cmd, (list, tuple)):
         cmd = shlex.split(cmd)
 
-    p = _popen_pipes(cmd, cwd)
-    out, err = p.communicate()
-    if out:
-        trace("out", repr(out))
-    if err:
-        trace("err", repr(err))
-    if p.returncode:
-        trace("ret", p.returncode)
+    res = _run(cmd, cwd)
+    if res.stdout:
+        trace("out", repr(res.stdout))
+    if res.stderr:
+        trace("err", repr(res.stderr))
+    if res.returncode:
+        trace("ret", res.returncode)
     return _t.CmdResult(
-        ensure_stripped_str(out), ensure_stripped_str(err), p.returncode
+        ensure_stripped_str(res.stdout), ensure_stripped_str(res.stderr), res.returncode
     )
 
 
@@ -105,23 +104,20 @@ def data_from_mime(path: _t.PathT) -> dict[str, str]:
     return data
 
 
-def function_has_arg(fn: object, argname: str) -> bool:
-    assert inspect.isfunction(fn)
-
-    argspec = inspect.signature(fn).parameters
-
-    return argname in argspec
+def function_has_arg(fn: object | FunctionType, argname: str) -> bool:
+    assert isinstance(fn, FunctionType)
+    code: CodeType = fn.__code__
+    return argname in code.co_varnames
 
 
 def has_command(name: str, args: list[str] | None = None, warn: bool = True) -> bool:
     try:
         cmd = [name, "help"] if args is None else [name, *args]
-        p = _popen_pipes(cmd, ".")
+        p = _run(cmd, ".")
     except OSError:
         trace(*sys.exc_info())
         res = False
     else:
-        p.communicate()
         res = not p.returncode
     if not res and warn:
         warnings.warn("%r was not found" % name, category=RuntimeWarning)

src/setuptools_scm/version.py

@@ -22,7 +22,7 @@
 from . import _types as _t
 from ._version_cls import Version as PkgVersion
 from .config import Configuration
-from .config import VersionT
+from .config import _VersionT
 from .utils import trace
 
 SEMVER_MINOR = 2
@@ -67,8 +67,8 @@ def callable_or_entrypoint(group: str, callable_or_name: str | Any) -> Any:
 
 
 def tag_to_version(
-    tag: VersionT | str, config: Configuration | None = None
-) -> VersionT | None:
+    tag: _VersionT | str, config: Configuration | None = None
+) -> _VersionT | None:
     """
     take a tag that might be prefixed with a keyword and return only the version part
     :param config: optional configuration object
@@ -101,13 +101,13 @@ def tag_to_version(
 
 def tags_to_versions(
     tags: list[str], config: Configuration | None = None
-) -> list[VersionT]:
+) -> list[_VersionT]:
     """
     take tags that might be prefixed with a keyword and return only the version part
     :param tags: an iterable of tags
     :param config: optional configuration object
     """
-    result: list[VersionT] = []
+    result: list[_VersionT] = []
     for tag in tags:
         parsed = tag_to_version(tag, config=config)
         if parsed:
@@ -192,8 +192,8 @@ def format_next_version(
 
 
 def _parse_tag(
-    tag: VersionT | str, preformatted: bool, config: Configuration | None
-) -> VersionT | str:
+    tag: _VersionT | str, preformatted: bool, config: Configuration | None
+) -> _VersionT | str:
     if preformatted:
         return tag
     elif config is None or not isinstance(tag, config.version_cls):
@@ -205,7 +205,7 @@ def _parse_tag(
 
 
 def meta(
-    tag: str | VersionT,
+    tag: str | _VersionT,
     distance: int | None = None,
     dirty: bool = False,
     node: str | None = None,

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist=py{36,37,38,39,310}-{test,selfcheck},check_readme,check-dist
+envlist=py{37,38,39,310}-{test,selfcheck},check_readme,check-dist
 
 [pytest]
 testpaths=testing