Use strict optional checking in glibc #12091
Conversation
Suggested by pradyunsg in pypa#11374 `--no-strict-optional` defeats half the purpose of using mypy. This change is trivial, we already catch AttributeError in the case that mypy is concerned about.
src/pip/_internal/utils/glibc.py
Outdated
| @@ -21,7 +18,7 @@ def glibc_version_string_confstr() -> Optional[str]: | |||
| return None | |||
| try: | |||
| # os.confstr("CS_GNU_LIBC_VERSION") returns a string like "glibc 2.17": | |||
| _, version = os.confstr("CS_GNU_LIBC_VERSION").split() | |||
| _, version = os.confstr("CS_GNU_LIBC_VERSION").split() # type: ignore[union-attr] | |||
There was a problem hiding this comment.
What is this ignoring? If it’s the None case, we should explicitly check for that instead
glibc_version = os.confstr("CS_GNU_LIBC_VERSION")
if glibc_version is None:
return None
...There was a problem hiding this comment.
It's ignoring the case where it returns None, because we already catch an AttributeError one line below
From https://docs.python.org/3/library/os.html#os.confstr
If the configuration value specified by name isn’t defined, None is returned
There was a problem hiding this comment.
I can change so we don't catch AttributeError though, just costs a line or two
edit: we still need to catch AttributeError because os.confstr is only available on Unix
There was a problem hiding this comment.
Theoratically Windows shouldn’t reach this line at all since there’s a check above (which is why Mypy does not complain). But it’s probably still best to keep the AttributeError just in case.
Suggested by pradyunsg in #11374
--no-strict-optionaldefeats half the purpose of using mypy.This change is trivial, just need a type ignore since we already catch AttributeError in the case that mypy is concerned about.