Merge pull request #1617 from bodograumann/improve-dx

Partial clean up wrt mypy and black

Author: Byron

Diff for: README.md

@@ -110,6 +110,8 @@ To typecheck, run: `mypy -p git`
 
 To test, run: `pytest`
 
+For automatic code formatting run: `black git`
+
 Configuration for flake8 is in the ./.flake8 file.
 
 Configurations for mypy, pytest and coverage.py are in ./pyproject.toml.

Diff for: git/__init__.py

@@ -76,7 +76,7 @@ def refresh(path: Optional[PathLike] = None) -> None:
     if not Git.refresh(path=path):
         return
     if not FetchInfo.refresh():
-        return
+        return  # type: ignore [unreachable]
 
     GIT_OK = True
 

Diff for: git/cmd.py

@@ -122,6 +122,7 @@ def handle_process_output(
         To specify a timeout in seconds for the git command, after which the process
         should be killed.
     """
+
     # Use 2 "pump" threads and wait for both to finish.
     def pump_stream(
         cmdline: List[str],
@@ -154,7 +155,7 @@ def pump_stream(
         p_stdout = process.proc.stdout if process.proc else None
         p_stderr = process.proc.stderr if process.proc else None
     else:
-        process = cast(Popen, process)
+        process = cast(Popen, process)  # type: ignore [redundant-cast]
         cmdline = getattr(process, "args", "")
         p_stdout = process.stdout
         p_stderr = process.stderr
@@ -210,7 +211,7 @@ def dashify(string: str) -> str:
     return string.replace("_", "-")
 
 
-def slots_to_dict(self: object, exclude: Sequence[str] = ()) -> Dict[str, Any]:
+def slots_to_dict(self: "Git", exclude: Sequence[str] = ()) -> Dict[str, Any]:
     return {s: getattr(self, s) for s in self.__slots__ if s not in exclude}
 
 
@@ -488,10 +489,7 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) ->
         """
         # Options can be of the form `foo` or `--foo bar` `--foo=bar`,
         # so we need to check if they start with "--foo" or if they are equal to "foo".
-        bare_unsafe_options = [
-            option.lstrip("-")
-            for option in unsafe_options
-        ]
+        bare_unsafe_options = [option.lstrip("-") for option in unsafe_options]
         for option in options:
             for unsafe_option, bare_option in zip(unsafe_options, bare_unsafe_options):
                 if option.startswith(unsafe_option) or option == bare_option:
@@ -1194,7 +1192,6 @@ def transform_kwargs(self, split_single_char_options: bool = True, **kwargs: Any
 
     @classmethod
     def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]:
-
         outlist = []
         if isinstance(arg_list, (list, tuple)):
             for arg in arg_list:

Diff for: git/config.py

@@ -248,7 +248,6 @@ def items_all(self) -> List[Tuple[str, List[_T]]]:
 
 
 def get_config_path(config_level: Lit_config_levels) -> str:
-
     # we do not support an absolute path of the gitconfig on windows ,
     # use the global config instead
     if is_win and config_level == "system":
@@ -265,8 +264,8 @@ def get_config_path(config_level: Lit_config_levels) -> str:
         raise ValueError("No repo to get repository configuration from. Use Repo._get_config_path")
     else:
         # Should not reach here. Will raise ValueError if does. Static typing will warn missing elifs
-        assert_never(
-            config_level,  # type: ignore[unreachable]
+        assert_never(  # type: ignore[unreachable]
+            config_level,
             ValueError(f"Invalid configuration level: {config_level!r}"),
         )
 
@@ -655,7 +654,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
             values: Sequence[str]  # runtime only gets str in tests, but should be whatever _OMD stores
             v: str
-            for (key, values) in section_dict.items_all():
+            for key, values in section_dict.items_all():
                 if key == "__name__":
                     continue
 

Diff for: git/diff.py

@@ -145,7 +145,7 @@ def diff(
         args.append("--full-index")  # get full index paths, not only filenames
 
         # remove default '-M' arg (check for renames) if user is overriding it
-        if not any(x in kwargs for x in ('find_renames', 'no_renames', 'M')):
+        if not any(x in kwargs for x in ("find_renames", "no_renames", "M")):
             args.append("-M")
 
         if create_patch:
@@ -338,7 +338,6 @@ def __init__(
         change_type: Optional[Lit_change_type],
         score: Optional[int],
     ) -> None:
-
         assert a_rawpath is None or isinstance(a_rawpath, bytes)
         assert b_rawpath is None or isinstance(b_rawpath, bytes)
         self.a_rawpath = a_rawpath

Diff for: git/exc.py

@@ -139,7 +139,6 @@ def __init__(
         valid_files: Sequence[PathLike],
         failed_reasons: List[str],
     ) -> None:
-
         Exception.__init__(self, message)
         self.failed_files = failed_files
         self.failed_reasons = failed_reasons
@@ -170,7 +169,6 @@ def __init__(
         stderr: Union[bytes, str, None] = None,
         stdout: Union[bytes, str, None] = None,
     ) -> None:
-
         super(HookExecutionError, self).__init__(command, status, stderr, stdout)
         self._msg = "Hook('%s') failed%s"
 

Diff for: git/index/base.py

@@ -656,7 +656,7 @@ def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry
     def _entries_for_paths(
         self,
         paths: List[str],
-        path_rewriter: Callable,
+        path_rewriter: Union[Callable, None],
         fprogress: Callable,
         entries: List[BaseIndexEntry],
     ) -> List[BaseIndexEntry]:

Diff for: git/index/fun.py

@@ -76,7 +76,7 @@ def hook_path(name: str, git_dir: PathLike) -> str:
     return osp.join(git_dir, "hooks", name)
 
 
-def _has_file_extension(path):
+def _has_file_extension(path: str) -> str:
     return osp.splitext(path)[1]
 
 
@@ -102,7 +102,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
             relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix()
             cmd = ["bash.exe", relative_hp]
 
-        cmd = subprocess.Popen(
+        process = subprocess.Popen(
             cmd + list(args),
             env=env,
             stdout=subprocess.PIPE,
@@ -116,13 +116,13 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
     else:
         stdout_list: List[str] = []
         stderr_list: List[str] = []
-        handle_process_output(cmd, stdout_list.append, stderr_list.append, finalize_process)
+        handle_process_output(process, stdout_list.append, stderr_list.append, finalize_process)
         stdout = "".join(stdout_list)
         stderr = "".join(stderr_list)
-        if cmd.returncode != 0:
+        if process.returncode != 0:
             stdout = force_text(stdout, defenc)
             stderr = force_text(stderr, defenc)
-            raise HookExecutionError(hp, cmd.returncode, stderr, stdout)
+            raise HookExecutionError(hp, process.returncode, stderr, stdout)
     # end handle return code
 
 
@@ -394,7 +394,6 @@ def aggressive_tree_merge(odb: "GitCmdObjectDB", tree_shas: Sequence[bytes]) ->
                         out.append(_tree_entry_to_baseindexentry(theirs, 0))
                     # END handle modification
                 else:
-
                     if ours[0] != base[0] or ours[1] != base[1]:
                         # they deleted it, we changed it, conflict
                         out.append(_tree_entry_to_baseindexentry(base, 1))

Diff for: git/objects/commit.py

@@ -345,9 +345,7 @@ def trailers(self) -> Dict[str, str]:
             Dictionary containing whitespace stripped trailer information.
             Only contains the latest instance of each trailer key.
         """
-        return {
-            k: v[0] for k, v in self.trailers_dict.items()
-        }
+        return {k: v[0] for k, v in self.trailers_dict.items()}
 
     @property
     def trailers_list(self) -> List[Tuple[str, str]]:
@@ -460,7 +458,7 @@ def _iter_from_process_or_stream(cls, repo: "Repo", proc_or_stream: Union[Popen,
             if proc_or_stream.stdout is not None:
                 stream = proc_or_stream.stdout
         elif hasattr(proc_or_stream, "readline"):
-            proc_or_stream = cast(IO, proc_or_stream)
+            proc_or_stream = cast(IO, proc_or_stream)  # type: ignore [redundant-cast]
             stream = proc_or_stream
 
         readline = stream.readline

Diff for: git/objects/fun.py

@@ -190,7 +190,6 @@ def traverse_trees_recursive(
     # is a tree. If the match is a non-tree item, put it into the result.
     # Processed items will be set None
     for ti, tree_data in enumerate(trees_data):
-
         for ii, item in enumerate(tree_data):
             if not item:
                 continue

Diff for: git/objects/util.py

@@ -143,11 +143,11 @@ def utctz_to_altz(utctz: str) -> int:
     :param utctz: git utc timezone string, i.e. +0200
     """
     int_utctz = int(utctz)
-    seconds = ((abs(int_utctz) // 100) * 3600 + (abs(int_utctz) % 100) * 60)
+    seconds = (abs(int_utctz) // 100) * 3600 + (abs(int_utctz) % 100) * 60
     return seconds if int_utctz < 0 else -seconds
 
 
-def altz_to_utctz_str(altz: int) -> str:
+def altz_to_utctz_str(altz: float) -> str:
     """Convert a timezone offset west of UTC in seconds into a git timezone offset string
 
     :param altz: timezone offset in seconds west of UTC

Diff for: git/remote.py

@@ -826,7 +826,6 @@ def _get_fetch_info_from_stderr(
         progress: Union[Callable[..., Any], RemoteProgress, None],
         kill_after_timeout: Union[None, float] = None,
     ) -> IterableList["FetchInfo"]:
-
         progress = to_progress_instance(progress)
 
         # skip first line as it is some remote info we are not interested in

Diff for: git/repo/base.py

@@ -498,7 +498,7 @@ def delete_head(self, *heads: "Union[str, Head]", **kwargs: Any) -> None:
     def create_tag(
         self,
         path: PathLike,
-        ref: Union[str, 'SymbolicReference'] = "HEAD",
+        ref: Union[str, "SymbolicReference"] = "HEAD",
         message: Optional[str] = None,
         force: bool = False,
         **kwargs: Any,
@@ -548,9 +548,8 @@ def _get_config_path(self, config_level: Lit_config_levels, git_dir: Optional[Pa
             else:
                 return osp.normpath(osp.join(repo_dir, "config"))
         else:
-
-            assert_never(
-                config_level,  # type:ignore[unreachable]
+            assert_never(  # type:ignore[unreachable]
+                config_level,
                 ValueError(f"Invalid configuration level: {config_level!r}"),
             )
 

Diff for: git/util.py

@@ -1049,7 +1049,7 @@ class IterableList(List[T_IterableObj]):
 
     __slots__ = ("_id_attr", "_prefix")
 
-    def __new__(cls, id_attr: str, prefix: str = "") -> "IterableList[IterableObj]":
+    def __new__(cls, id_attr: str, prefix: str = "") -> "IterableList[T_IterableObj]":
         return super(IterableList, cls).__new__(cls)
 
     def __init__(self, id_attr: str, prefix: str = "") -> None:
@@ -1083,7 +1083,6 @@ def __getattr__(self, attr: str) -> T_IterableObj:
         return list.__getattribute__(self, attr)
 
     def __getitem__(self, index: Union[SupportsIndex, int, slice, str]) -> T_IterableObj:  # type: ignore
-
         assert isinstance(index, (int, str, slice)), "Index of IterableList should be an int or str"
 
         if isinstance(index, int):
@@ -1098,7 +1097,6 @@ def __getitem__(self, index: Union[SupportsIndex, int, slice, str]) -> T_Iterabl
         # END handle getattr
 
     def __delitem__(self, index: Union[SupportsIndex, int, slice, str]) -> None:
-
         assert isinstance(index, (int, str)), "Index of IterableList should be an int or str"
 
         delindex = cast(int, index)

Diff for: pyproject.toml

@@ -19,6 +19,7 @@ filterwarnings = 'ignore::DeprecationWarning'
 # filterwarnings ignore::WarningType  # ignores those warnings
 
 [tool.mypy]
+python_version = "3.7"
 disallow_untyped_defs = true
 no_implicit_optional = true
 warn_redundant_casts = true
@@ -29,6 +30,7 @@ implicit_reexport = true
 # strict = true
 
 # TODO: remove when 'gitdb' is fully annotated
+exclude = ["^git/ext/gitdb"]
 [[tool.mypy.overrides]]
 module = "gitdb.*"
 ignore_missing_imports = true
@@ -43,3 +45,4 @@ omit = ["*/git/ext/*"]
 [tool.black]
 line-length = 120
 target-version = ['py37']
+exclude = "git/ext/gitdb"