search

skorokithakis / shortuuid

A generator library for concise, unambiguous and URL-safe UUIDs.
http://www.stavros.io/
BSD 3-Clause "New" or "Revised" License
2.05k stars 109 forks source link

feat: Add type hinting #77

Closed pcolladosoto closed 2 years ago

pcolladosoto commented 2 years ago

We drew inspiration from the library and in doing so we came across #75. The least we can do is try to give a bit back, so we've added type hints throughout the code.

We have tried to do so adhering to the overall style of the code when it came to importing new libraries and we decided to stick with typing to support a wider range of Python versions when it comes to type hinting as a whole.

We know we might have missed some points and that some annotations can be better expressed in some way or another. That's why we would really appreciate any feedback: we'll be happy to include it!

Before opening this PR we ran the tests to verify we hadn't broken anything.

Hope you find this little bit of work useful 😸!

skorokithakis commented 2 years ago

Excellent, thank you very much! I'll review soon and comment, thanks again!

pcolladosoto commented 2 years ago

Looks good, thank you! I'm always wondering whether we should have a type alias for generated UUIDs so they're a ShortUUID type, instead of str (but aliased to str), but I don't know if that'll mess up something else down the line...

The first language I learned was C and I have always been an advocate for keeping types as close to the basic ones as possible. I'd rather know I'm getting a str back than some custom type... However, type aliasing can indeed be an elegant solution.

I also don't know how used people are to type hinting in Python: I guess a ton of people like it because it's dynamically typed in the first place. These folks could maybe get caught up in a web of aliases and custom types that doesn't really bring that much to the table...

In the end I think it's mostly a matter of taste really. It's always nice to know what possible choices we have and to ponder which might be the most suitable.

In any case, I'm more than happy to lend a (small) hand. Thank you for your work: you saved me a ton of time! 😼

skorokithakis commented 2 years ago

Thanks for the PR! I disagree about keeping types close to the basic ones, as then you lose a lot of the guarantees (if a number is in feet and another in meters but they're both ints, the compiler won't warn you when you try to use one as the other), but I agree that it doesn't add much if you have to convert every time.

I'll think about it some more, and I've merged and will release this in the mean time, thank you for your help!

pcolladosoto commented 2 years ago

Thank you for the insight on data types! It's always great to get to know what other fellow developers think about such topics: I find it tremendously enriching 😄

Once again, thanks a ton for your wonderful work: I'm glad I could help a little 😸

skorokithakis commented 2 years ago

You helped a lot, thank you! I love types, and forgot that ShortUUID doesn't have them.