unicorn icon indicating copy to clipboard operation
unicorn copied to clipboard

Modify python version check such that linters recognize it

Open omer54463 opened this issue 1 year ago • 1 comments

This is sort of a follow up to #2005. Currently many type checkers have a hard time recognizing all the different ways you can compare versions with sys.version_info (it's a best-effort kind of thing).

I changed the comparison and made sure that:

  1. It works for both python 3 and python 2.7.
  2. Mypy and Pyright both understand the comparison and skip the python 2.7 implementation.

omer54463 avatar Oct 18 '24 12:10 omer54463

Personally I like less the half-tuple comparison (i.e. (3, )). What wrong with the current method? I don't see any problems with MyPy or Pylint on my side.

elicn avatar Oct 19 '24 20:10 elicn

Here's what I'm experiencing:

uc = unicorn.Uc(unicorn.UC_ARCH_X86, unicorn.UC_MODE_64)
reveal_type(uc) # Revealed type is "unicorn.unicorn_py2.Uc"mypy(note)

Which aligns with what Mypy claims:

Mypy supports the ability to perform Python version checks and platform checks (e.g. Windows vs Posix), ignoring code paths that won’t be run on the targeted Python version or platform. This allows you to more effectively typecheck code that supports multiple versions of Python or multiple operating systems.

More specifically, mypy will understand the use of sys.version_info and sys.platform checks within if/elif/else statements. For example:

import sys

# Distinguishing between different versions of Python:
if sys.version_info >= (3, 8):
    # Python 3.8+ specific definitions and imports
else:
    # Other definitions and imports

# Distinguishing between different operating systems:
if sys.platform.startswith("linux"):
    # Linux-specific code
elif sys.platform == "darwin":
    # Mac-specific code
elif sys.platform == "win32":
    # Windows-specific code
else:
    # Other systems

...

Mypy currently does not support more complex checks, and does not assign any special meaning when assigning a sys.version_info or sys.platform check to a variable. This may change in future versions of mypy.

omer54463 avatar Oct 23 '24 23:10 omer54463

@omer54463 Is it intended to close this?

wtdcode avatar Dec 07 '24 14:12 wtdcode