search

lmas / opensimplex

This repo has been migrated to https://code.larus.se/lmas/opensimplex
MIT License
241 stars 29 forks source link

Add full typing #29

Closed mfbehrens closed 2 years ago

lmas commented 2 years ago

Thanks, been wanting this for a while 🤟 I'm busy until sometime next week so it's going to take a while for me to review.

-- Alex

On Wed, 9 Feb 2022, at 01:54, Michael Behrens wrote:

You can view, comment on, or merge this pull request online at:

https://github.com/lmas/opensimplex/pull/29

Commit Summary

(2 files https://github.com/lmas/opensimplex/pull/29/files)

lmas commented 2 years ago

On 18 February 2022 12:14:38 UTC, Michael Behrens @.> wrote: @. commented on this pull request.

I think type hinting is not encouraging people to use the functions. After all, what is the problem of people using the internal functions. Moreover, it is better for ourselves to have the code type hinted for finding potential bugs.

The type hint docs mentions that it's not enforced by the language, it's up to the user to use tools that will check the type hints. No gain if you don't use the tools (I'm currently not enforcing the linter because the tooling won't play nice yet, haven't made sure if it even checks typing).

The docs I linked to had a deprecation warning for typing.Tuple, so I don't want to merge commit a4a8b83 as there's a higher risk (compared to not having type hints) that it might break this library in the future when Python drops it completely.

The way I see it, the issue still stands? We need another solution.

lmas commented 2 years ago

Eh I finally decided to not bother with any quirks and simply removed the type hint for that func for now :shrug:

Edit: Holding the merge until the other PR is done, as it's smaller than this one.

lmas commented 2 years ago

After more consideration and some research, I'm still not convinced of any usefulness of adding type hints to the internals and still think it's just adding unnecessary complexity/line noise with little to no gain at this time.

On the other hand, I gladly accept type hints for the public API as they can serve as a usage hints and hopefully make it clearer how to use the API.

lmas commented 2 years ago

Thank you for work!