Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork947
Description
Theversion_info
property is declared to be a length-4 tuple:
Lines 833 to 841 inafa5754
@property | |
defversion_info(self)->Tuple[int,int,int,int]: | |
""" | |
:return: tuple(int, int, int, int) tuple with integers representing the major, minor | |
and additional version numbers as parsed from git version. | |
This value is generated on demand and is cached. | |
""" | |
returnself._version_info |
But it often has fewer values, which is intentional:
Lines 814 to 826 inafa5754
def_set_cache_(self,attr:str)->None: | |
ifattr=="_version_info": | |
# We only use the first 4 numbers, as everything else could be strings in fact (on Windows). | |
process_version=self._call_process("version")# Should be as default *args and **kwargs used. | |
version_numbers=process_version.split(" ")[2] | |
self._version_info=cast( | |
Tuple[int,int,int,int], | |
tuple(int(n)forninversion_numbers.split(".")[:4]ifn.isdigit()), | |
) | |
else: | |
super()._set_cache_(attr) | |
# END handle version info |
So the type annotation should be changed, but I am unsure what it should be changed to, whether any additional documentation should be added, and whether having fewer than some number of numeric fields should be treated as an error and cause an exception to be raised.
If the type annotation should reflect that only a few specific lengths are reasonable, then it could be expressed as aUnion
of specific-lengthTuple
types. Otherwise it could be expressed as a variable-length tuple by writing the type asTuple[int, ...]
.
I recommend this be solved in such a way that thecast
can be removed. Currently, thecast
is working around ambiguity in how the return type is intended to be understood.
This is separate from#1829, though I noticed it while looking into that and they could possibly be fixed (or improved or otherwise closed) together.