Description
Since #1659, git.__all__
is written literally, rather than being dynamically constructed based on what has been defined at the time it is bound. However, the fix was based on what __all__
had been being populated with at the time, and therefore inherited two weaknesses:
- Lots of imports that don't make sense to use from the
git
module (because they are from the standard library) are included. This is a fairly minor issue, but I suggest marking them as deprecated. ProbablyDeprecationWarning
should not be raised when accessing them, though, because although people should not use wildcard imports in general, such warnings when using them in a REPL could obscure more important deprecation warnings. git.refresh
isn't public, at least not by the convention that when__all__
exists everything excluded from it is non-public. This is mitigated by the references in the documentation that say to use it and to prefer it to theGit.refresh
method, and by its inclusion in the autodoc-generated API reference page, but it would still be best for it to be listed ingit.__all__
. The reason it isn't is that, back when__all__
had been constructed dynamically, this was above thedef
forrefresh
.
I've included a fix for this in #1859, since I was adding to __all__
for #1874 already. Because PEP-8 recommends __all__
come before most code in a module, and the reason that couldn't be done before was that it was being built dynamically and so had to be defined below everything that was to be included in it, I went ahead and did that too. As with #1874, in view of the grown scope of #1859, it seemed clearer to have an issue for this that #1859 lists as fixing, rather than jam it into comments there.
The other aspect of the situation with git.__all__
following #1659 is the question of whether its content really has been stable. After all, it was constructed from wildcard imports from modules whose contents sometimes changed and not all of which had __all__
attributes, and so potentially could have changed heavily across releases. If it had, then it might be acceptable to remove things that are only there for compatibility (because: compatibility with what?) and also would be necessary to add at least a commented warning when making other substantial changes, to avoid obscuring the issue.
Fortunately, that is not the case. It has been remarkably stable. I wrote a script to check and the results can be checked here. (I actually did this around the time of #1659, but there was nothing to raise an alarm about, and the matter didn't come up again until now.)