search

BurnySc2 / python-sc2

A StarCraft II bot api client library for Python 3
MIT License
509 stars 157 forks source link

Question: Library typing aprroach #117

Closed mac2000 closed 2 years ago

mac2000 commented 2 years ago

Hello, just having fun and playing with a library, trying to see what can I achieve with it

Seems like runtime errors costs way too much here, e.g. after line change you start over, wait few minutes and then it crashed because you tried to call non callable something 🤷‍♂️

Thankfully PyCharm here is to help at least to catch some of issues before running the code

But seems like it does not able to figure things our correctly, here and there I see PyCharm complaining about wrong usage but if I try to do what is suggested I got runtime error, e.g.:

image

Here for example, it says that worker.position is a function which returns Point2 but if I try worker.position() i will get non callable exception

image

Once again, complaining about wrong usage and suggesting to execute method

With this things I can not rely on PyCharm and forced to rerun everything each time to check if everything is fine which takes too much time for things that technically should be statically checked

Wondering if this is something that can be fixed in future or it is by intent and we should do our best to explore right way to do things on our own?

image

even more concrete example, thankfully error message is self explanatory enough

BurnySc2 commented 2 years ago

Here for example, it says that worker.position is a function which returns Point2 but if I try worker.position() i will get non callable exception

Unsure why it does that. It uses a custom property which tries to cache it (which hopefully runs faster than a normal @propery decorator): https://github.com/BurnySc2/python-sc2/blob/b595bbe1a5b252f49fb60592742d2750c5fc6df6/sc2/unit.py#L512-L515

With this things I can not rely on PyCharm and forced to rerun everything each time to check if everything is fine which takes too much time for things that technically should be statically checked

Mypy checks got added slowly over time - I haven't implemented them everywhere. Building Refinery / Assimilator / Extractor is a special case as it is the only building that requires the target to be a unit instead of a position, sadly.

For this case we added a special function, perhaps this helps you: https://github.com/BurnySc2/python-sc2/blob/b595bbe1a5b252f49fb60592742d2750c5fc6df6/sc2/unit.py#L1257-L1280

I just noticed the docstrings are not quite perfect.

BurnySc2 commented 2 years ago

Fixed in https://github.com/BurnySc2/python-sc2/pull/131 where I use @cached_property from the functools library, which should have the proper type hints. This requires Python 3.8+ though.