René's URL Explorer Experiment

Title: Use zero-argument super() by EliahKagan · Pull Request #1726 · gitpython-developers/GitPython · GitHub

Open Graph Title: Use zero-argument super() by EliahKagan · Pull Request #1726 · gitpython-developers/GitPython

X Title: Use zero-argument super() by EliahKagan · Pull Request #1726 · gitpython-developers/GitPython

Description: This replaces 2-argument super(...) calls with the zero-argument super() form everywhere applicable: All uses of two-argument super in the code are replaced. All but one mention of two-argument super in comments are replaced. I had omitted this from #1725 because it may require more technical attention to fully check than other code changes there, and that PR was mostly for comments and docstrings. Exactly when super() can be used Since super is in practice one of the most confusing features of Python, I've included this section to help in verifying the changes here, and also so that they could be reverified later. However, this section is optional. As noted in output of help(super) (which is from this code, though the other documentation can be useful): super() -> same as super(__class__, ) __class__ is actually literal there--though the static __class__ cell should not be confused with the dynamic __class__ attribute--but it can also be thought of as standing for the name of the enclosing class (or narrowest enclosing class in the case of nested classes), which is actually what one typically writes in the two-argument form of super. When the expression passed in place of __class__ refers to the (narrowest enclosing) class, and the expression passed in place of is the name of the first parameter of the method (typically self in instance methods, or cls in class methods or __new__), the zero-argument form super() can be used instead with the same meaning. The one place I couldn't replace it The above-described situation applied everywhere in GitPython where super was actually used. However, there are also comments in some places describing how something else could be done or how what was done could be done differently. In those, most showings of multiple-argument super were replaceable with super without changing what the code would mean and do if used in the place the comment pertained to. But then there is this, in git.objects.util.TraversableIterableObj: GitPython/git/objects/util.py Lines 625 to 658 in 6cef9c0 def traverse( self: T_TIobj, predicate: Callable[[Union[T_TIobj, TIobj_tuple], int], bool] = lambda i, d: True, prune: Callable[[Union[T_TIobj, TIobj_tuple], int], bool] = lambda i, d: False, depth: int = -1, branch_first: bool = True, visit_once: bool = True, ignore_self: int = 1, as_edge: bool = False, ) -> Union[Iterator[T_TIobj], Iterator[Tuple[T_TIobj, T_TIobj]], Iterator[TIobj_tuple]]: """For documentation, see util.Traversable._traverse()""" """ # To typecheck instead of using cast. import itertools from git.types import TypeGuard def is_commit_traversed(inp: Tuple) -> TypeGuard[Tuple[Iterator[Tuple['Commit', 'Commit']]]]: for x in inp[1]: if not isinstance(x, tuple) and len(x) != 2: if all(isinstance(inner, Commit) for inner in x): continue return True ret = super(Commit, self).traverse(predicate, prune, depth, branch_first, visit_once, ignore_self, as_edge) ret_tup = itertools.tee(ret, 2) assert is_commit_traversed(ret_tup), f"{[type(x) for x in list(ret_tup[0])]}" return ret_tup[0] """ return cast( Union[Iterator[T_TIobj], Iterator[Tuple[Union[None, T_TIobj], T_TIobj]]], super(TraversableIterableObj, self)._traverse( predicate, prune, depth, branch_first, visit_once, ignore_self, as_edge # type: ignore ), ) The actual super call in the code there is replaceable, and I replaced it, with zero-argument super(). But the comment--or, more precisely, the string being used as a comment--shows a call with Commit as the class name. I'm not sure if that's intended or not. If it is intended, maybe it represents code that would be written somewhere else. So I haven't changed that, though I'm willing to do so if the matter is clarified. It could be changed either in a new commit in this PR, or in a subsequent PR. Other changes There's some proofreading of the kind done in #1725 that I missed there and took care of here. More significantly, TestBigRepoR.setUp contains: GitPython/test/performance/lib.py Lines 35 to 38 in 6cef9c0 try: super(TestBigRepoR, self).setUp() except AttributeError: pass And TestBigRepoRW.setUp analogously contains: GitPython/test/performance/lib.py Lines 67 to 70 in 6cef9c0 try: super(TestBigRepoRW, self).setUp() except AttributeError: pass In addition to replacing those two-argument super calls with zero-argument super(), I also removed the logic to catch and swallow AttributeError. As covered in more detail in the 9113177 commit message, this is unnecessary because unittest.TestCase defines an empty setUp method to allow this kind of special casing to be avoided, and it may be brittle because AttributeError could be raised from the actual code of a setUp method, due to the specific way the code to swallow it is written. However, if that latter effect, which would usually be strongly unwanted, is actually the goal, then this should be undone and a suitable comment and/or clarifying refactoring done. What isn't included It's unclear whether super calls in test classes' setUp and tearDown methods are always made in the places they are made because that is where superclass and sibling-class fixture acquisition and release behavior should occur. For example, usually, if setUp delegates via super to another class's setUp method at the beginning, and in a case where the corresponding tearDown method needs delegate via super to another class's tearDown method, then that tearDown call should be at the end, so that resource acquisition and release more often occurs in a temporally nested way that is easier to reason about and more often correct. However, sometimes doing it differently is deliberate. Furthermore, setUp and tearDown methods are inherently weird, because, if a subclass's setUp calls a superclass's setUp and then fails with an exception, then tearDown won't run. (Also, in situations where tearDown does run, an exception in one tearDown method may still prevent other MRO classes' tearDown methods from running. Unlike the first issue, for this there isn't really any perfect thing to do. But I think the framework still makes it somewhat harder than necessary to reason about when the tearDown calls are manually written rather than arranged and implied by the setUp logic.) For this reason, I am unsure if it is really worthwhile to try to improve ordering. If improvements are to be made, it might be best to have only setUp methods and have them register cleanup logic by calling addCleanup--or, perhaps better where applicable, enterContext, though a backport of it for Python <3.11 would be needed. Unlike tearDown methods, cleanup scheduled with addCleanup, either directly or through enterContext, is deterministically performed, in the opposite of the order it was added, even if setUp failed with an exception after the addCleanup call.

Open Graph Description: This replaces 2-argument super(...) calls with the zero-argument super() form everywhere applicable: All uses of two-argument super in the code are replaced. All but one mention of two-argument su...

X Description: This replaces 2-argument super(...) calls with the zero-argument super() form everywhere applicable: All uses of two-argument super in the code are replaced. All but one mention of two-argument su...

Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1726

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width