ivankorobkov / python-inject

Python dependency injection
Apache License 2.0
672 stars 77 forks source link

Type-hints are separated from implementation #31

Closed andrewborba10 closed 5 years ago

andrewborba10 commented 5 years ago

Type-hints are separated into a .pyi file alongside inject.py. This seems unnecessary when they can be bundled together for easier integration. Right now, my IDE doesn't understand the type-hints by default, and while I'm sure I can fiddle with settings and get it to pick up the .pyi file (which is something I've had trouble with in the past) its just a hassle that gets passed on to every new consumer of this library.

I believe .pyi files were introduced as a stop-gap for the Python standard library to allow for incremental additions from the community and generally should be avoided in scenarios like this where the implementation can be fairly easily updated.

I would gladly volunteer to do the work on this (in fact, I already wrote some type-hints within our own project via a shim between our code and this library, but that approach has some short-falls that ultimately motivated me to come here).

Please let me know if you're on board with this idea and I can submit a pull request.

andrewborba10 commented 5 years ago

I'm done with the changes, let me know how you would want to integrate them! I can push a new branch and to this repo if you give me write permission and then I can submit a pull request. Thanks!

While I did the typing annotations in a Python 2-compatible way, it will still require the typing module to be available on the module path. Hopefully this is ok.

ivankorobkov commented 5 years ago

Hi, can it break Python2 compatibility somehow? If not, please make a pull request.

andrewborba10 commented 5 years ago

This change shouldn't break Python 2 compatibility.

EDIT: Didn't realize Github had a very easy forking UX, so I forked the repo and will submit pull requests shortly.

ivankorobkov commented 5 years ago

I have merged your pull request. Thanks. Please, close the issue if it's done.

ivankorobkov commented 5 years ago

Should I release a patch version?

andrewborba10 commented 5 years ago

Sure, that would be great, thanks!