Skip to content

Replace all wildcard imports with explicit imports #1880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 34 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
1e5a944
Add a script to validate refactored imports
EliahKagan Mar 17, 2024
5b2771d
Add regression tests of the git.util aliasing situation
EliahKagan Mar 18, 2024
fc86a23
Incompletely change git.index imports to test modattrs.py
EliahKagan Feb 24, 2024
4badc19
Fix git.index imports
EliahKagan Mar 18, 2024
1c9bda2
Improve relative order of import groups, and __all__, in git.index
EliahKagan Mar 18, 2024
8b51af3
Improve order of imports and __all__ in git.refs submodules
EliahKagan Mar 18, 2024
b25dd7e
Replace wildcard imports in git.refs
EliahKagan Mar 18, 2024
b32ef65
Improve order of imports and __all__ in git.repo submodules
EliahKagan Mar 18, 2024
0ba06e9
Add git.repo.__all__ and make submodules explicit
EliahKagan Mar 18, 2024
c946906
Improve order of imports and __all__ in git.objects.*
EliahKagan Mar 18, 2024
4e9a2f2
Improve order of imports and __all__ in git.object.submodule.*
EliahKagan Mar 18, 2024
c58be4c
Remove a bit of old commented-out code in git.objects.*
EliahKagan Mar 18, 2024
01c95eb
Don't patch IndexObject and Object into git.objects.submodule.util
EliahKagan Mar 18, 2024
f89d065
Fix git.objects.__all__ and make submodules explicit
EliahKagan Mar 18, 2024
3786307
Make git.objects.util module docstring more specific
EliahKagan Mar 18, 2024
de540b7
Add __all__ and imports in git.objects.submodule
EliahKagan Mar 18, 2024
a05597a
Improve how imports and __all__ are written in git.util
EliahKagan Mar 18, 2024
2053a3d
Remove old commented-out change_type assertions in git.diff
EliahKagan Mar 18, 2024
b8bab43
Remove old commented-out flagKeyLiteral assertions in git.remote
EliahKagan Mar 19, 2024
3d4e476
Improve how second-level imports and __all__ are written
EliahKagan Mar 19, 2024
6318eea
Make F401 "unused import" suppressions more specific
EliahKagan Mar 19, 2024
31bc8a4
Remove unneeded F401 "Unused import" suppressions
EliahKagan Mar 19, 2024
abbe74d
Fix a tiny import sorting nit
EliahKagan Mar 19, 2024
7745250
Replace wildcard imports in top-level git module
EliahKagan Mar 19, 2024
64c9efd
Restore relative order to fix circular import error
EliahKagan Mar 19, 2024
31f89a1
Add the nonpublic indirect submodule aliases back for now
EliahKagan Mar 19, 2024
9bbbcb5
Further improve git.objects.util module docstring
EliahKagan Mar 19, 2024
00f4cbc
Add missing submodule imports in git.objects
EliahKagan Mar 19, 2024
fcc7418
Don't explicitly list direct submodules in __all__
EliahKagan Mar 19, 2024
78055a8
Pick a consistent type for __all__ (for now, list)
EliahKagan Mar 19, 2024
ecdb6aa
Save diff of non-__all__ attributes across import changes
EliahKagan Mar 19, 2024
f705fd6
Remove modattrs.py and related
EliahKagan Mar 19, 2024
4a4d880
Improve test suite import grouping/sorting, __all__ placement
EliahKagan Mar 19, 2024
d524c76
Fix slightly unsorted imports in setup.py
EliahKagan Mar 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Incompletely change git.index imports to test modattrs.py
This also checks if the regression tests in test_imports.py work.

This replaces wildcard imports in git.index with explicit imports,
and adds git.index.__all__. But this temporarily omits from it the
public attributes of git.index that name its Python submodules and
are thus are present as an indirect effect of importing names
*from* them (since doing so also imports them).

This partial change, until it is completed in the next commit,
represents the kind of bug that modattrs.py seeks to safeguard
against, and verifies that a diff between old and current output of
modattrs.py clearly shows it. This diff is temporarily included as
ab.diff, which will be deleted in the next commit.

The key problem that diff reveals is the changed value of the util
attribute of the top-level git module. Due to the way wildcard
imports have been used within GitPython for its own modules,
expressions like `git.util` (where `git` is the top-level git
module) and imports like `from git import util` are actually
referring to git.index.util, rather than the util Python submodule
of the git module (conceptually git.util), which can only be
accessed via `from git.util import ...` or in `sys.modules`.
Although this is not an inherently good situation, in practice it
has to be maintained at least until the next major version, because
reasonable code that uses GitPython would be broken if the
situation were changed to be more intuitive.

But the more intuitive behavior automatically happens if wildcard
imports are replaced with explicit imports of the names they had
originally intended to import (in this case, in the top-level git
module, which has not yet been done but will be), or if __all__ is
added where helpful (in this case, in git.index, which this does).
Adding the names explicitly is sufficient to restore the old
behavior that is needed for backward compatibility, and has the
further advantage that the unintuitive behavior will be explicit
once all wildcard imports are replaced and __all__ is added to each
module where it would be helpful. The modattrs.py script serves to
ensure incompatible changes are not made; this commit checks it.

The tests in test_imports.py also cover this specific anticipated
incompatibility in git.util, but not all breakages that may arise
when refactoring imports and that diff-ing modattrs.py output would
help catch.
  • Loading branch information
EliahKagan committed Mar 18, 2024
commit fc86a238f99a6406ed949a7d690ec0f6be2f31e0
30 changes: 30 additions & 0 deletions ab.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
diff --git a/a b/b
index 81b3f984..7b8c8ede 100644
--- a/a
+++ b/b
@@ -83,14 +83,12 @@ git:
__path__: ['C:\\Users\\ek\\source\\repos\\GitPython\\git']
__spec__: ModuleSpec(name='git', loader=<_frozen_importlib_external.SourceFileLoader object at 0x...>, origin='C:\\Users\\ek\\source\\repos\\GitPython\\git\\__init__.py', submodule_search_locations=['C:\\Users\\ek\\source\\repos\\GitPython\\git'])
__version__: 'git'
- base: <module 'git.index.base' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\index\\base.py'>
cmd: <module 'git.cmd' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\cmd.py'>
compat: <module 'git.compat' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\compat.py'>
config: <module 'git.config' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\config.py'>
db: <module 'git.db' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\db.py'>
diff: <module 'git.diff' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\diff.py'>
exc: <module 'git.exc' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\exc.py'>
- fun: <module 'git.index.fun' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\index\\fun.py'>
head: <module 'git.refs.head' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\refs\\head.py'>
index: <module 'git.index' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\index\\__init__.py'>
log: <module 'git.refs.log' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\refs\\log.py'>
@@ -106,9 +104,8 @@ git:
symbolic: <module 'git.refs.symbolic' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\refs\\symbolic.py'>
tag: <module 'git.refs.tag' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\refs\\tag.py'>
to_hex_sha: <function to_hex_sha at 0x...>
- typ: <module 'git.index.typ' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\index\\typ.py'>
types: <module 'git.types' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\types.py'>
- util: <module 'git.index.util' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\index\\util.py'>
+ util: <module 'git.util' from 'C:\\Users\\ek\\source\\repos\\GitPython\\git\\util.py'>


git.cmd:
13 changes: 11 additions & 2 deletions git/index/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,14 @@

"""Initialize the index package."""

from .base import * # noqa: F401 F403
from .typ import * # noqa: F401 F403
__all__ = [
"BaseIndexEntry",
"BlobFilter",
"CheckoutError",
"IndexEntry",
"IndexFile",
"StageType",
]

from .base import CheckoutError, IndexFile
from .typ import BaseIndexEntry, BlobFilter, IndexEntry, StageType
5 changes: 2 additions & 3 deletions git/index/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
"""Module containing :class:`IndexFile`, an Index implementation facilitating all kinds
of index manipulations such as querying and merging."""

__all__ = ("IndexFile", "CheckoutError", "StageType")

import contextlib
import datetime
import glob
Expand Down Expand Up @@ -81,9 +83,6 @@
# ------------------------------------------------------------------------------------


__all__ = ("IndexFile", "CheckoutError", "StageType")


@contextlib.contextmanager
def _named_temporary_file_for_subprocess(directory: PathLike) -> Generator[str, None, None]:
"""Create a named temporary file git subprocesses can open, deleting it afterward.
Expand Down
5 changes: 2 additions & 3 deletions git/index/typ.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

"""Additional types used by the index."""

__all__ = ("BlobFilter", "BaseIndexEntry", "IndexEntry", "StageType")

from binascii import b2a_hex
from pathlib import Path

from .util import pack, unpack
from git.objects import Blob


# typing ----------------------------------------------------------------------

from typing import NamedTuple, Sequence, TYPE_CHECKING, Tuple, Union, cast
Expand All @@ -23,8 +24,6 @@

# ---------------------------------------------------------------------------------

__all__ = ("BlobFilter", "BaseIndexEntry", "IndexEntry", "StageType")

# { Invariants
CE_NAMEMASK = 0x0FFF
CE_STAGEMASK = 0x3000
Expand Down