From e54277fc868a521e360dd9eba5f643321088e813 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Dec 2023 18:59:36 -0500 Subject: [PATCH 1/3] Compare types in test_orig_head with "is" This performs an exact type comparison with "is" instead of "==". Most (maybe all?) of the rest of the places where "==" was used have been changed previously, but this on fell through the cracks. Like the None object, type objects use reference-based equality comparison, and the idiomatic ways to check against them are "is" for exactly comparisons and isinstance/issubclass otherwise. --- test/test_refs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_refs.py b/test/test_refs.py index 6ee385007..2656ceab1 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -207,7 +207,7 @@ def test_is_valid(self): assert not SymbolicReference(self.rorepo, "hellothere").is_valid() def test_orig_head(self): - assert type(self.rorepo.head.orig_head()) == SymbolicReference + assert type(self.rorepo.head.orig_head()) is SymbolicReference @with_rw_repo("0.1.6") def test_head_checkout_detached_head(self, rw_repo): From cf5cb8472fa74834274e7077b4c703783ed5112e Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Dec 2023 19:04:27 -0500 Subject: [PATCH 2/3] Small docstring copyedits - Slightly improve grammar ("it's" -> "its"). - Add a missing space after a "." (between sentences). - Use more consistent hard wrap in a place where 88 was intended. --- git/objects/submodule/base.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 9ed1487a5..acf16b556 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -958,7 +958,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = raise # END handle undo rename - # Auto-rename submodule if it's name was 'default', that is, the checkout directory. + # Auto-rename submodule if its name was 'default', that is, the checkout directory. if previous_sm_path == self.name: self.rename(module_checkout_path) @@ -976,19 +976,19 @@ def remove( from the .gitmodules file and the entry in the .git/config file. :param module: If True, the checked out module we point to will be deleted as - well.If that module is currently on a commit outside any branch in the + well. If that module is currently on a commit outside any branch in the remote, or if it is ahead of its tracking branch, or if there are modified - or untracked files in its working tree, then the removal will fail. - In case the removal of the repository fails for these reasons, the - submodule status will not have been altered. + or untracked files in its working tree, then the removal will fail. In case + the removal of the repository fails for these reasons, the submodule status + will not have been altered. If this submodule has child modules of its own, these will be deleted prior to touching the direct submodule. :param force: Enforces the deletion of the module even though it contains modifications. This basically enforces a brute-force file system based deletion. :param configuration: If True, the submodule is deleted from the configuration, - otherwise it isn't. Although this should be enabled most of the time, - this flag enables you to safely delete the repository of your submodule. + otherwise it isn't. Although this should be enabled most of the time, this + flag enables you to safely delete the repository of your submodule. :param dry_run: If True, we will not actually do anything, but throw the errors we would usually throw. :return: self From 41d22b277cb0866b3a742e7de63bc33aa881dd04 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 8 Dec 2023 19:08:57 -0500 Subject: [PATCH 3/3] Overhaul noqa directives - Remove some noqa directives that are not needed at all anymore. But there may still be some others I did not find. - Fix the code and remove the noqa direcive in places where the change is very simple and does not affect the interface (e.g., what could be taken to be public or not, what symbols are available and what they refer to, etc.), the change makes the code more idiomatic or clearer, and it fixes the flake8 complaint completely. - Turn file-level noqa directives into statement-level ones. A noqa directive at file level, even if it lists one or more specific error/warning numbers, actually suppresses flake8 for the entire file. So the main benefit here is to enable other warnings. But this also makes it clear what is producing the ones we suppress, which makes it easier to know what is intentional and how it may be reasonable to change it in the future. - Move noqa directives at the end of multi-line imports to the top line, where flake8 honors them, and eliminate redundant directives that were added to work around their ineffectiveness. (This is not really separate from the above point, since this is often what allowed file-level directives to be removed.) - Specify the errors/warnings each "noqa" should suppress. That is, this turns every bare "noqa", aside from those eliminated as described above, into a "noqa" for one or more specific errors/warnings. This makes the intent clearer and avoids oversuppression. - Add missing ":" between "noqa" and error/warning numbers. When these were absent, the intention was to suppress only the specifically listed errors/warnings, but the effect was that the listed numbers were ignored and *everything* was suppressed. In a couple of cases it was necessary to change the listed numbers to correct the list to what actually needed to be suppressed. - Write every "noqa" in lower-case (not "NOQA"). There did not appear to be a systematic reason for the different casing, and having them all cased the same makes it easier to avoid mistakes when grepping for them. - Where an import is unused and does not appear intended to be accesed from other modules, and is present only to support code in the same module that is commented out, but whose removal should be considered separately, comment out the statement or part of the statement that imports it as well, rather than writing a "noqa" for the unused import. This applies to only one file, git/objects/util.py. --- git/__init__.py | 27 +++++++++++++-------------- git/cmd.py | 2 +- git/compat.py | 9 ++------- git/index/__init__.py | 6 ++---- git/objects/__init__.py | 20 +++++++++----------- git/objects/util.py | 22 +++++++++------------- git/refs/__init__.py | 13 ++++++------- git/refs/log.py | 4 ++-- git/refs/reference.py | 4 ++-- git/refs/symbolic.py | 5 ++--- git/repo/__init__.py | 4 +--- git/types.py | 14 ++++++-------- git/util.py | 8 ++++---- test/lib/__init__.py | 4 ++-- test/lib/helper.py | 4 ++-- test/test_commit.py | 2 +- test/test_config.py | 2 +- test/test_docs.py | 2 +- test/test_exc.py | 14 +++++++------- test/test_repo.py | 2 +- test/test_submodule.py | 2 +- 21 files changed, 75 insertions(+), 95 deletions(-) diff --git a/git/__init__.py b/git/__init__.py index defc679cb..c6a52ef30 100644 --- a/git/__init__.py +++ b/git/__init__.py @@ -3,28 +3,27 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -# flake8: noqa # @PydevCodeAnalysisIgnore -from git.exc import * # @NoMove @IgnorePep8 -from typing import List, Optional, Sequence, Tuple, Union, TYPE_CHECKING -from git.types import PathLike - __version__ = "git" +from typing import List, Optional, Sequence, Tuple, Union, TYPE_CHECKING + from gitdb.util import to_hex_sha +from git.exc import * # noqa: F403 # @NoMove @IgnorePep8 +from git.types import PathLike try: from git.compat import safe_decode # @NoMove @IgnorePep8 from git.config import GitConfigParser # @NoMove @IgnorePep8 - from git.objects import * # @NoMove @IgnorePep8 - from git.refs import * # @NoMove @IgnorePep8 - from git.diff import * # @NoMove @IgnorePep8 - from git.db import * # @NoMove @IgnorePep8 + from git.objects import * # noqa: F403 # @NoMove @IgnorePep8 + from git.refs import * # noqa: F403 # @NoMove @IgnorePep8 + from git.diff import * # noqa: F403 # @NoMove @IgnorePep8 + from git.db import * # noqa: F403 # @NoMove @IgnorePep8 from git.cmd import Git # @NoMove @IgnorePep8 from git.repo import Repo # @NoMove @IgnorePep8 - from git.remote import * # @NoMove @IgnorePep8 - from git.index import * # @NoMove @IgnorePep8 + from git.remote import * # noqa: F403 # @NoMove @IgnorePep8 + from git.index import * # noqa: F403 # @NoMove @IgnorePep8 from git.util import ( # @NoMove @IgnorePep8 LockFile, BlockingLockFile, @@ -33,12 +32,12 @@ remove_password_if_present, rmtree, ) -except GitError as _exc: +except GitError as _exc: # noqa: F405 raise ImportError("%s: %s" % (_exc.__class__.__name__, _exc)) from _exc # __all__ must be statically defined by py.typed support # __all__ = [name for name, obj in locals().items() if not (name.startswith("_") or inspect.ismodule(obj))] -__all__ = [ +__all__ = [ # noqa: F405 "Actor", "AmbiguousObjectName", "BadName", @@ -127,7 +126,7 @@ def refresh(path: Optional[PathLike] = None) -> None: if not Git.refresh(path=path): return - if not FetchInfo.refresh(): + if not FetchInfo.refresh(): # noqa: F405 return # type: ignore [unreachable] GIT_OK = True diff --git a/git/cmd.py b/git/cmd.py index d67bb1478..8b42dec52 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -965,7 +965,7 @@ def execute( # Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value. maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1") else: - cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable + cmd_not_found_exception = FileNotFoundError maybe_patch_caller_env = contextlib.nullcontext() # END handle diff --git a/git/compat.py b/git/compat.py index 64bd38397..6b1b4f547 100644 --- a/git/compat.py +++ b/git/compat.py @@ -5,20 +5,15 @@ """Utilities to help provide compatibility with Python 3.""" -# flake8: noqa - import locale import os import sys -from gitdb.utils.encoding import ( - force_bytes, # @UnusedImport - force_text, # @UnusedImport -) +from gitdb.utils.encoding import force_bytes, force_text # noqa: F401 # @UnusedImport # typing -------------------------------------------------------------------- -from typing import ( +from typing import ( # noqa: F401 Any, AnyStr, Dict, diff --git a/git/index/__init__.py b/git/index/__init__.py index 9954b9e88..c65722cd8 100644 --- a/git/index/__init__.py +++ b/git/index/__init__.py @@ -3,7 +3,5 @@ """Initialize the index package.""" -# flake8: noqa - -from .base import * -from .typ import * +from .base import * # noqa: F401 F403 +from .typ import * # noqa: F401 F403 diff --git a/git/objects/__init__.py b/git/objects/__init__.py index 9ca430285..fc8d8fbb2 100644 --- a/git/objects/__init__.py +++ b/git/objects/__init__.py @@ -3,23 +3,21 @@ """Import all submodules' main classes into the package space.""" -# flake8: noqa - import inspect -from .base import * -from .blob import * -from .commit import * +from .base import * # noqa: F403 +from .blob import * # noqa: F403 +from .commit import * # noqa: F403 from .submodule import util as smutil -from .submodule.base import * -from .submodule.root import * -from .tag import * -from .tree import * +from .submodule.base import * # noqa: F403 +from .submodule.root import * # noqa: F403 +from .tag import * # noqa: F403 +from .tree import * # noqa: F403 # Fix import dependency - add IndexObject to the util module, so that it can be # imported by the submodule.base. -smutil.IndexObject = IndexObject # type: ignore[attr-defined] -smutil.Object = Object # type: ignore[attr-defined] +smutil.IndexObject = IndexObject # type: ignore[attr-defined] # noqa: F405 +smutil.Object = Object # type: ignore[attr-defined] # noqa: F405 del smutil # Must come after submodule was made available. diff --git a/git/objects/util.py b/git/objects/util.py index 3e8f8dcc9..c52dc7564 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -5,20 +5,16 @@ """General utility functions.""" -# flake8: noqa F401 - - from abc import ABC, abstractmethod -import warnings -from git.util import IterableList, IterableObj, Actor - -import re +import calendar from collections import deque - +from datetime import datetime, timedelta, tzinfo from string import digits +import re import time -import calendar -from datetime import datetime, timedelta, tzinfo +import warnings + +from git.util import IterableList, IterableObj, Actor # typing ------------------------------------------------------------ from typing import ( @@ -26,10 +22,10 @@ Callable, Deque, Iterator, - Generic, + # Generic, NamedTuple, overload, - Sequence, # NOQA: F401 + Sequence, TYPE_CHECKING, Tuple, Type, @@ -38,7 +34,7 @@ cast, ) -from git.types import Has_id_attribute, Literal, _T # NOQA: F401 +from git.types import Has_id_attribute, Literal # , _T if TYPE_CHECKING: from io import BytesIO, StringIO diff --git a/git/refs/__init__.py b/git/refs/__init__.py index 3a82b9796..b0233e902 100644 --- a/git/refs/__init__.py +++ b/git/refs/__init__.py @@ -1,13 +1,12 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -# flake8: noqa # Import all modules in order, fix the names they require. -from .symbolic import * -from .reference import * -from .head import * -from .tag import * -from .remote import * +from .symbolic import * # noqa: F401 F403 +from .reference import * # noqa: F401 F403 +from .head import * # noqa: F401 F403 +from .tag import * # noqa: F401 F403 +from .remote import * # noqa: F401 F403 -from .log import * +from .log import * # noqa: F401 F403 diff --git a/git/refs/log.py b/git/refs/log.py index aeebac48c..e45798d8a 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -31,9 +31,9 @@ from git.types import PathLike if TYPE_CHECKING: - from git.refs import SymbolicReference from io import BytesIO - from git.config import GitConfigParser, SectionConstraint # NOQA + from git.refs import SymbolicReference + from git.config import GitConfigParser, SectionConstraint # ------------------------------------------------------------------------------ diff --git a/git/refs/reference.py b/git/refs/reference.py index c2ad13bd6..20d42472d 100644 --- a/git/refs/reference.py +++ b/git/refs/reference.py @@ -10,8 +10,8 @@ # typing ------------------------------------------------------------------ -from typing import Any, Callable, Iterator, Type, Union, TYPE_CHECKING # NOQA -from git.types import Commit_ish, PathLike, _T # NOQA +from typing import Any, Callable, Iterator, Type, Union, TYPE_CHECKING +from git.types import Commit_ish, PathLike, _T if TYPE_CHECKING: from git.repo import Repo diff --git a/git/refs/symbolic.py b/git/refs/symbolic.py index 84c2057e1..31f959ac2 100644 --- a/git/refs/symbolic.py +++ b/git/refs/symbolic.py @@ -1,7 +1,6 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -from git.types import PathLike import os from git.compat import defenc @@ -31,8 +30,8 @@ Union, TYPE_CHECKING, cast, -) # NOQA -from git.types import Commit_ish, PathLike # NOQA +) +from git.types import Commit_ish, PathLike if TYPE_CHECKING: from git.repo import Repo diff --git a/git/repo/__init__.py b/git/repo/__init__.py index c01a1e034..a63d77878 100644 --- a/git/repo/__init__.py +++ b/git/repo/__init__.py @@ -3,6 +3,4 @@ """Initialize the Repo package.""" -# flake8: noqa - -from .base import Repo as Repo +from .base import Repo as Repo # noqa: F401 diff --git a/git/types.py b/git/types.py index 6f2b7c513..efb393471 100644 --- a/git/types.py +++ b/git/types.py @@ -1,11 +1,9 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -# flake8: noqa - import os import sys -from typing import ( +from typing import ( # noqa: F401 Dict, NoReturn, Sequence as Sequence, @@ -16,24 +14,24 @@ Callable, TYPE_CHECKING, TypeVar, -) # noqa: F401 +) if sys.version_info >= (3, 8): - from typing import ( + from typing import ( # noqa: F401 Literal, TypedDict, Protocol, SupportsIndex as SupportsIndex, runtime_checkable, - ) # noqa: F401 + ) else: - from typing_extensions import ( + from typing_extensions import ( # noqa: F401 Literal, SupportsIndex as SupportsIndex, TypedDict, Protocol, runtime_checkable, - ) # noqa: F401 + ) # if sys.version_info >= (3, 10): # from typing import TypeGuard # noqa: F401 diff --git a/git/util.py b/git/util.py index aaf662060..0a5da7d71 100644 --- a/git/util.py +++ b/git/util.py @@ -62,12 +62,9 @@ Has_id_attribute, ) -T_IterableObj = TypeVar("T_IterableObj", bound=Union["IterableObj", "Has_id_attribute"], covariant=True) -# So IterableList[Head] is subtype of IterableList[IterableObj] - # --------------------------------------------------------------------- -from gitdb.util import ( # NOQA @IgnorePep8 +from gitdb.util import ( # noqa: F401 # @IgnorePep8 make_sha, LockedFD, # @UnusedImport file_contents_ro, # @UnusedImport @@ -79,6 +76,9 @@ hex_to_bin, # @UnusedImport ) +T_IterableObj = TypeVar("T_IterableObj", bound=Union["IterableObj", "Has_id_attribute"], covariant=True) +# So IterableList[Head] is subtype of IterableList[IterableObj]. + # NOTE: Some of the unused imports might be used/imported by others. # Handle once test-cases are back up and running. # Most of these are unused here, but are for use by git-python modules so these diff --git a/test/lib/__init__.py b/test/lib/__init__.py index cc1e48483..f96072cb5 100644 --- a/test/lib/__init__.py +++ b/test/lib/__init__.py @@ -3,8 +3,8 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -# flake8: noqa import inspect -from .helper import * + +from .helper import * # noqa: F401 F403 __all__ = [name for name, obj in locals().items() if not (name.startswith("_") or inspect.ismodule(obj))] diff --git a/test/lib/helper.py b/test/lib/helper.py index f4e22245b..58d96534a 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -145,7 +145,7 @@ def repo_creator(self): os.chdir(rw_repo.working_dir) try: return func(self, rw_repo) - except: # noqa E722 + except: # noqa: E722 B001 log.info("Keeping repo after failure: %s", repo_dir) repo_dir = None raise @@ -305,7 +305,7 @@ def remote_repo_creator(self): with cwd(rw_repo.working_dir): try: return func(self, rw_repo, rw_daemon_repo) - except: # noqa E722 + except: # noqa: E722 B001 log.info( "Keeping repos after failure: \n rw_repo_dir: %s \n rw_daemon_repo_dir: %s", rw_repo_dir, diff --git a/test/test_commit.py b/test/test_commit.py index b6fb09aef..fddb91c17 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -475,7 +475,7 @@ def test_datetimes(self): commit.authored_datetime, datetime(2009, 10, 8, 18, 17, 5, tzinfo=tzoffset(-7200)), commit.authored_datetime, - ) # noqa + ) self.assertEqual( commit.authored_datetime, datetime(2009, 10, 8, 16, 17, 5, tzinfo=utc), diff --git a/test/test_config.py b/test/test_config.py index c1b26c91f..f493c5672 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -125,7 +125,7 @@ def test_multi_line_config(self): ev = "ruby -e '\n" ev += " system %(git), %(merge-file), %(--marker-size=%L), %(%A), %(%O), %(%B)\n" ev += " b = File.read(%(%A))\n" - ev += " b.sub!(/^<+ .*\\nActiveRecord::Schema\\.define.:version => (\\d+). do\\n=+\\nActiveRecord::Schema\\." # noqa E501 + ev += " b.sub!(/^<+ .*\\nActiveRecord::Schema\\.define.:version => (\\d+). do\\n=+\\nActiveRecord::Schema\\." # noqa: E501 ev += "define.:version => (\\d+). do\\n>+ .*/) do\n" ev += " %(ActiveRecord::Schema.define(:version => #{[$1, $2].max}) do)\n" ev += " end\n" diff --git a/test/test_docs.py b/test/test_docs.py index 2f4b2e8d8..a03a9c71b 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -25,7 +25,7 @@ def tearDown(self): # # @skipIf(HIDE_WINDOWS_KNOWN_ERRORS, # "FIXME: helper.wrapper fails with: PermissionError: [WinError 5] Access is denied: " - # "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa E501 + # "'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\test_work_tree_unsupportedryfa60di\\master_repo\\.git\\objects\\pack\\pack-bc9e0787aef9f69e1591ef38ea0a6f566ec66fe3.idx") # noqa: E501 @with_rw_directory def test_init_repo_object(self, rw_dir): # [1-test_init_repo_object] diff --git a/test/test_exc.py b/test/test_exc.py index cecd4f342..3f4d0b803 100644 --- a/test/test_exc.py +++ b/test/test_exc.py @@ -40,13 +40,13 @@ ), ) _causes_n_substrings = ( - (None, None), # noqa: E241 @IgnorePep8 - (7, "exit code(7)"), # noqa: E241 @IgnorePep8 - ("Some string", "'Some string'"), # noqa: E241 @IgnorePep8 - ("παλιο string", "'παλιο string'"), # noqa: E241 @IgnorePep8 - (Exception("An exc."), "Exception('An exc.')"), # noqa: E241 @IgnorePep8 - (Exception("Κακια exc."), "Exception('Κακια exc.')"), # noqa: E241 @IgnorePep8 - (object(), "