RemoteProgress._parse_progress_line is conceptually a "protected" method: code outside GitPython may subclass RemoteProgress and override it with another implementation (which may also reasonably call the base class implementation).
The method is conceptually void as implemented in the base class, always implicitly returning None, and it is annotated accordingly:
|
def _parse_progress_line(self, line: AnyStr) -> None: |
However, while it would not ordinarily be useful to do so, it is--or has been--also intended that it be overridable to return a non-None value. This is mostly for compatibility with existing code that may have done so in the past, but it may be that reasonable cases for doing so in new code can still arise. This is reflected in code of GitPython:
|
def new_message_handler(self) -> Callable[[str], None]: |
|
""" |
|
:return: |
|
A progress handler suitable for handle_process_output(), passing lines on to |
|
this Progress handler in a suitable format |
|
""" |
|
|
|
def handler(line: AnyStr) -> None: |
|
return self._parse_progress_line(line.rstrip()) |
|
|
|
# END handler |
|
|
|
return handler |
And in the subclass of RemoteProgress introduced in the test suite, which overrides the method:
|
def _parse_progress_line(self, line): |
|
# We may remove the line later if it is dropped. |
|
# Keep it for debugging. |
|
self._seen_lines.append(line) |
|
rval = super()._parse_progress_line(line) |
|
return rval |
But any override that returns a non-None value is currently inconsistent with the return type annotation on the base class, which is written as -> None. For Liskov substitutability, an overridden method's return type must be the same or a subclass of the base method's return type, and if a subclass has type annotations, and the return type is specified as a non-Any type, type checkers such as mypy and pyright will report a non-None return type as a violation.
If possible, the return type in RemoteProgress should be broadened, but in a way that does not unduly encourage subclasses to return non-None. See comments on #1788, starting at #1788 (comment), for a discussion and analysis of this. This issue, and the above recommendation, is based on the findings and discussion there.