ethpm / py-ethpm

This library is deprecated. ethPM python tooling is now located in web3.py
MIT License
24 stars 13 forks source link

Add type hints #30

Closed njgheorghita closed 6 years ago

njgheorghita commented 6 years ago

What was wrong?

There were no type-hints in this repo && type-hints are tight

How was it fixed?

Implemented type hints && mypy

Cute Animal Picture

image

closes #29

njgheorghita commented 6 years ago

Thanks for the suggestions @cburgdorf I think it's a good idea to be more restrictive. made the changes here, going to go ahead and merge and address @pipermerriam comments in a new pr

cburgdorf commented 6 years ago

@njgheorghita great job! Super happy to see us having strong type support here right from the start :heart:

I've send a minor nitpick correction converting a comment type annotation to a proper one: https://github.com/ethpm/py-ethpm/pull/31

Assuming that we don't need to be compatible with Python 3.5, we don't need to use comment type annotations which have several disadvantages over proper ones.

njgheorghita commented 6 years ago

👍 @cburgdorf Can you rattle off a couple advantages that having strong type support gives us? is it more for the dev - understanding the inputs/outputs - or are there also performance benefits?

cburgdorf commented 6 years ago

Sure! This is just my personal biased view and order but here it goes:

  1. It eliminates lots of bugs. For instance, you can not accidentally push an int into a List[str] which then blows up at runtime. Bugs like this have literally killed NASA missions in which APIs were called with kilometers where miles where expected. Btw, this is a great use case for NewTypes where you can tighten APIs that normally would be simply typed to accept int but through a NewType can be locked down to really only accept a mile as a substitute for an int. Types simply prevent these kind of bugs at development time.

  2. Tooling. Types make it possible to develop better tooling because the tools have more information about the code statically available. This allows for better autocompletes, refactoring support etc.

  3. Documentation. APIs become much more readable and descriptive especially also for people who are new to the code base.

Consider this def.

def apply_child_computation(self, child_msg: Message) -> BaseComputation

Just from looking at the signature I get a much better understanding of what the method does without having to parse and compile the code in my head.

Compare that with

def apply_child_computation(self, child_msg)

I can't even tell that this method returns something without digging into the code. Even better, with types my IDE can quickly jump to BaseCompution making it much easier to explore the code base.

or are there also performance benefits

There are no performance benefits at least for this kind of gradual typing that modern Python or TypeScript (as an enhancement of JavaScript) gives you. This kind of typing is basically invisible at runtime and is strictly about improving the developer experience.

However, now that I think about it, there is in fact a way where this kind of typing can indirectly help you to improve the performance and it is in fact something that @pipermerriam and friends came to discuss here. The gist of it is that currently a lot of Python code uses runtime assertions to at least ensure at runtime that certain APIs are only called with certain types. But of course, these kind of runtime type checks have a cost so if you can get the same level of confidence at development time you can simply throw away those runtime checks and get a performance gain.

That said, right now type checking is still something that one has to configure through mypy as a standalone tool but given the success of TypeScript I'm pretty sure the Python community will adopt types quickly and I bet that in the future mypy will simply converge into the regular python interpreter. At some point you'll be able to pass --type-check to Python and won't need a separate tool like mypy anymore. And soon after that you'll have to pass --disable-type-checks to disable type checking because it will be enabled by default :sweat_smile:

njgheorghita commented 6 years ago

@cburgdorf Sick! Thanks so much for the write-up!!