Fix dynamically-set __all__ variable by DeflateAwning · Pull Request #1659 · gitpython-developers/GitPython

@DeflateAwning

Byron

requested changes Sep 13, 2023

@DeflateAwning

- explicit imports in exc added to avoid linting errors in __init__

@DeflateAwning

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Oct 11, 2023
…opers#1659

When gitpython-developers#1659 was updated to pick up linting configuration changes, it
inadvertently undid one of the URL changes made in gitpython-developers#1662, putting
the URL in the git.exe module back to the one that redirects to a
different BSD license from the one this project uses.

Since only that one module was affected, the fix is simple. This
only changes the URL back; it doesn't undo any other gitpython-developers#1659 changes.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Oct 11, 2023
…opers#1659

When gitpython-developers#1659 was updated to pick up linting configuration changes, it
inadvertently undid one of the URL changes made in gitpython-developers#1662, putting
the URL in the git.exc module back to the one that redirects to a
different BSD license from the one this project uses.

Since only that one module was affected, the fix is simple. This
only changes the URL back; it doesn't undo any other gitpython-developers#1659 changes.

Byron added a commit that referenced this pull request

Oct 12, 2023

This was referenced

Oct 12, 2023

This was referenced

Oct 18, 2023

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Oct 20, 2023
The git.exc module imports exceptions from gitdb.exc to republish
them, as well as defining its own (also for use from outside). But
because it did not define __all__, the intent for the exceptions it
imported was unclear, since names that are introduced by imports
and not present in __all__ are not generally considered public,
even when __all__ is absent and a "*" import would reimport them.

This rectifies that by adding __all__ and listing both imported and
newly introduced exceptions explicitly in it. Although this
strictly expands which names are public under typical conventions,
it strictly contracts which names are imported by a "*" import,
because the presence of __all__ suppresses names not listed in it
from being imported that way. However, because under typical
conventions those other names are not considered public, and they
were not even weakly documented as public, this should be okay.

(Even though this is not a breaking change, in that code it would
break would already technically be broken... if it turns out that
it is common to wrongly rely on the availabiliy of those names,
then this may need to be revisited and slightly modified.)

This brings the readily identified public interface of git.exc in
line with what is weakly implied (and intended) by its docstring.

This also modifies __init__.py accordingly: The top-level git
module has for some time used a "*" import on git.exc, causing
the extra names originally meant as implementation details to be
included. Because its own __all__ was dynamically generated until
c862845, gitpython-developers#1659 also added 8edc53b to retain the formerly present
names in __all__. So the change here imports those names from the
modules that deliberately provide them, to preserve compatibility.

This was referenced

Oct 20, 2023

renovate bot referenced this pull request in allenporter/flux-local

Oct 20, 2023

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Feb 24, 2024
This makes the git.refresh function unambiguously public.

git.refresh was already public in the sense that it was explicitly
documented as appropriate to call from code outside GitPython.
However, it had not been included in git.__all__.

Because __all__ existed but omitted "refresh", git.refresh had
appeared non-public to automated tools.

This also does some cleanup:

- It removes a comment that showed how git.__all__ had been defined
  dynamically before gitpython-developers#1659, since with the addition of "refresh",
  git.__all__ no longer contains exactly the same elements as that
  technique produced (as it examined the module's contents prior to
  running the def statement that bound the name "refresh").

- With that comment removed, it is no longer necessary to define
  __all__ below the imports to show what the dynamic techinque had
  operated on. So this moves it up above them in accordance with
  PEP-8.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Feb 24, 2024
This adds comments to entries in git.__all__ for each of the
entries that come from the standard library typing module, noting
them as deprecated.

These imports were included in __all__ inadvertently due to the
way __all__ was dynamically constructed, and placed in __all__
explicitly when __all__ became static in gitpython-developers#1659. They are there for
backward compatibility, in case some code relies on them being
there. But a module is unlikely to rely intentionally on the git
module providing them, since they are not conceptually related to
GitPython.

`from git import *` should not typically be used, since wildcard
imports are not generally recommended, as discussed in PEP-8. But
if someone does choose to use it, they would probably benefit less
from DeprecationWarning being issued for each of those names than
they would usually benefit from DeprecationWarning. This could lead
to developers deciding not to enable DeprecationWarning when it may
otherwise be useful. For this reason, no attempt is currently made
to issue DeprecationWarning when those names are accessed as
attributes of the git module.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 5, 2024
This makes the git.refresh function unambiguously public.

git.refresh was already public in the sense that it was explicitly
documented as appropriate to call from code outside GitPython.
However, it had not been included in git.__all__.

Because __all__ existed but omitted "refresh", git.refresh had
appeared non-public to automated tools.

This also does some cleanup:

- It removes a comment that showed how git.__all__ had been defined
  dynamically before gitpython-developers#1659, since with the addition of "refresh",
  git.__all__ no longer contains exactly the same elements as that
  technique produced (as it examined the module's contents prior to
  running the def statement that bound the name "refresh").

- With that comment removed, it is no longer necessary to define
  __all__ below the imports to show what the dynamic techinque had
  operated on. So this moves it up above them in accordance with
  PEP-8.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 5, 2024
This adds comments to entries in git.__all__ for each of the
entries that come from the standard library typing module, noting
them as deprecated.

These imports were included in __all__ inadvertently due to the
way __all__ was dynamically constructed, and placed in __all__
explicitly when __all__ became static in gitpython-developers#1659. They are there for
backward compatibility, in case some code relies on them being
there. But a module is unlikely to rely intentionally on the git
module providing them, since they are not conceptually related to
GitPython.

`from git import *` should not typically be used, since wildcard
imports are not generally recommended, as discussed in PEP-8. But
if someone does choose to use it, they would probably benefit less
from DeprecationWarning being issued for each of those names than
they would usually benefit from DeprecationWarning. This could lead
to developers deciding not to enable DeprecationWarning when it may
otherwise be useful. For this reason, no attempt is currently made
to issue DeprecationWarning when those names are accessed as
attributes of the git module.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 9, 2024
This makes the git.refresh function unambiguously public.

git.refresh was already public in the sense that it was explicitly
documented as appropriate to call from code outside GitPython.
However, it had not been included in git.__all__.

Because __all__ existed but omitted "refresh", git.refresh had
appeared non-public to automated tools.

This also does some cleanup:

- It removes a comment that showed how git.__all__ had been defined
  dynamically before gitpython-developers#1659, since with the addition of "refresh",
  git.__all__ no longer contains exactly the same elements as that
  technique produced (as it examined the module's contents prior to
  running the def statement that bound the name "refresh").

- With that comment removed, it is no longer necessary to define
  __all__ below the imports to show what the dynamic techinque had
  operated on. So this moves it up above them in accordance with
  PEP-8.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 9, 2024
This adds comments to entries in git.__all__ for each of the
entries that come from the standard library typing module, noting
them as deprecated.

These imports were included in __all__ inadvertently due to the
way __all__ was dynamically constructed, and placed in __all__
explicitly when __all__ became static in gitpython-developers#1659. They are there for
backward compatibility, in case some code relies on them being
there. But a module is unlikely to rely intentionally on the git
module providing them, since they are not conceptually related to
GitPython.

`from git import *` should not typically be used, since wildcard
imports are not generally recommended, as discussed in PEP-8. But
if someone does choose to use it, they would probably benefit less
from DeprecationWarning being issued for each of those names than
they would usually benefit from DeprecationWarning. This could lead
to developers deciding not to enable DeprecationWarning when it may
otherwise be useful. For this reason, no attempt is currently made
to issue DeprecationWarning when those names are accessed as
attributes of the git module.

This was referenced

Mar 11, 2024

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 18, 2024
The submodules being made explicit here are of course Python
submodules, not git submodules.

The git.objects.submodule Python submodule (which is *about* git
submodules) is made explicit here (as are the names imported from
that Python submodule's own Python submodules) along with the other
Python submodules of git.objects.

Unlike some other submodules, but like the top-level git module
until gitpython-developers#1659, git.objects already defined __all__ but it was
dynamically constructed. As with git.__all__ previously (as noted
in gitpython-developers#1859), I have used https://github.com/EliahKagan/deltall here
to check that it is safe, sufficient, and probably necessary to
replace this dynamically constructed __all__ with a literal list of
strings in which all of the original elements are included. See:
https://gist.github.com/EliahKagan/e627603717ca5f9cafaf8de9d9f27ad7

Running the modattrs.py script, and diffing against the output from
before the current import refactoring began, reveals two changes to
module contents as a result of the change here:

- git.objects no longer contains `inspect`, which it imported just
  to build the dynamically constructed __all__. Because this was
  not itself included in that __all__ or otherwise made public or
  used out of git.objects, that is okay. This is exactly analogous
  to the situation in 8197e90, which removed it from the top-level
  git module after gitpython-developers#1659.

- The top-level git module now has has new attributes blob, commit,
  submodule, and tree, aliasing the corresponding modules. This
  has happened as a result of them being put in git.objects.__all__
  along with various names imported from them. (As noted in some
  prior commits, there are tradeoffs associated with doing this,
  and it may be that such elements of __all__ should be removed,
  here and elsewhere.)