You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Have git module use sys.platform to check for Windows
This changes the way code throughout the git module checks to see
if it is running on a native Windows system.
Some checks using os.name were already changed to use sys.platform
in dc95a76 and 4191f7d. (See dc95a76 on why the specific question
of whether the system is native Windows can be answered by either
checking os.name or sys.platform, even though in general they
differ in granularity and are not always suited to the same tasks.)
The reasons for this change are:
- Consistency: Due to dc95a76 and 4191f7d, some of these checks use
sys.platform, so in the absence of a reason to do otherwise, it
is best that they all do. Otherwise, it creates an appearance
that the technical reasons behind the difference are stronger
than they really are, or even that differnt things are being
checked.
- Better static typing: mypy treats sys.platform as a constant
(and also allows checking on platform on another via --platform).
Likewise, typeshed conditions platform-dependent declarations on
sys.platform checks, rather than os.name or other checks. Really
this is the original reason for those earlier, more selective
changes, but here the goal is more general, since this is not
needed to address any specific preexisting mypy errors.
This is incomplete, for two reasons:
- I'm deliberately not including changes in the tests in this
commit. Arguably it should be kept as os.name in the tests, on
the grounds that the test suite is not currently statically typed
anyway, plus having them differ more compellingly shows that the
behavior is the same whether an os.name or sys.platform check is
used. However, it would be confusing to keep them different, and
also somewhat unnatural; one approach would probably end up
leaking through. Furthermore, because some tests have to check
for cygwin specifically, which cannot be done with os.name, it
may be clearer to use sys.platform for all platform checking in
the test suite. But to verify and demonstrate that the change
really is safe, I'm waiting to make the change in the tests until
after making them in the code under test.
- Some forms of checks against constants produce unreachable code
errors from mypy (python/mypy#10773).
This can be worked around, but I have not done that in this
commit. Furthermore, the new mypy errors this produces--one on
Windows, and one on non-Windows systems--could be fixed in a way
that makes type annotations richer, by allowing the return type
to be a literal on one platform or the other.
0 commit comments