Open Emerentius opened 5 years ago
Interesting, thanks for sharing. I'm not familiar with mypy, can you elaborate on some of these benefits?
Optional static typing gives users much better autocompletion and the ability to use mypy for statically checking their code
Autocompletion in what context? And, statically checking pyseq code, or client code? Are pydocs insufficient here? This is not a very large project, and it doesn't seem like these changes will improve the performance, so I'm a little unclear what the benefits are yet.
Also, if static typing is the goal why not look into Cython, which will also improve performance?
Autocompletion is improved in IDEs like VS Code and PyCharm. When the editor knows that a variable is a str
or a list
for example, it can suggest methods of those types as soon as you type a dot. In my experience, this makes using libraries a much nicer experience.
A type checker like mypy or pyright can also warn when you're passing the wrong type to a function (like a str
where an int
is expected) or calling methods that the type doesn't support. The <variable> may be None
warnings I quoted are also examples of this.
Both pyseq and client code can be statically checked. For clients, type checkers will assure that users are passing the right types arguments and inform them of what the exact types are that they are getting back. Documentation doesn't obviate types. It's more effort to consult it and it is also often outdated or incomplete. Case in point: uncompress
claims to return a Sequence
, but it can also return None
or an empty list. The documentation for walk
doesn't tell you the return type at all.
Type checkers help by automating correctness checks, their goal in Python isn't performance. Type annotations don't affect runtime behaviour at all. I don't know about Cython, but these types are a simple, fully compatible add-on onto regular Python. They have received special syntax in Python3 any many libraries are supporting them already.
This adds type hints via the python2 compatible comment syntax. Optional static typing gives users much better autocompletion and the ability to use
mypy
for statically checking their code, if they so choose. There is no downside for users who don't want to make use of it and no runtime costs except for a singleif False
check during imports.There are no code changes except for:
str = str
because it trips up mypyreturn
withreturn None
in one casereturn [ext] + _natural_key(name)
toif False
at the top block where I'm importing the necessary types for mypy. Although it never executes at runtime, mypy understands the intent.I've tried to make the type hints as general as possible so that there aren't any false positives. Two warnings remain, at least one of which is because of a bug in the code. I've also uncovered some weird cases.
Notes on type hints
List[str]
arguments. BecauseList[T]
isn't covariant overT
, i.e. aList[str]
argument won't accept aList[Item]
. I had to fallback toList[Any]
in those cases.AnyStr
for any argument that acceptsbasestring
without mixingstr
andbytes
. Because these are used for path operations,os.PathLike
would better show the intent, but that is not available in Python2.Strange behaviour / inconsistencies encountered
Item
andSequence
. Doing so invariably causes errors because both of these classes violate the Liskov substitution principle.Item
can't be used everywhere astr
can andSequence
can't be used everywhere alist
can.uncompress()
has a weird return type, it's eitherNone
, an emptyList
or a nonemptySequence
. Not necessarily wrong, just strange.isinstance(arg, basestring)
butiget_sequences
checksisinstance(…, str)
. Therefore I annotated the argument asstr
as well. Thewalk()
function on the other hand claims to takebasestring
but uses them together withstr
literals which will crash when combined withbytes
in Python3. I've usedstr
type hints there.Remaining warnings
Line 656: item.frame may be None
For some reason, self.frame is only updated in
is_sibling()
which may be called inincludes()
. That doesn't happen, when theSequence
contains only a single element -> Crash.Line 1013:
s
ande
may beNone
.The function is complicated, so I can't tell at a glance if
s
ande
are guaranteed to be set at this point. If they are, then that can either beassert
ed just before use or the warning deactivated via# type: ignore