Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix aes argument typehint according to PEP0484 #409

Closed
wants to merge 1 commit into from
Closed

fix aes argument typehint according to PEP0484 #409

wants to merge 1 commit into from

Conversation

@has2k1
Copy link
Owner

has2k1 commented Jun 17, 2020

I see the mix-up. kwargs: dict in the docstring means that kwargs is a dict, it does not say anything about the type of keys or values in the dict. The libraries in the scientific python ecosystem document keyword parameters the same way. This is different from pep-0484 were you give the type hint of the keyword parameters.

I wonder, is an IDE is interpreting kwargs: dict as a type hint for the values?

@FFroehlich
Copy link
Author

I see the mix-up. kwargs: dict in the docstring means that kwargs is a dict, it does not say anything about the type of keys or values in the dict. The libraries in the scientific python ecosystem document keyword parameters the same way. This is different from pep-0484 were you give the type hint of the keyword parameters.

I wonder, is an IDE is interpreting kwargs: dict as a type hint for the values?

Correct, PyCharm is interpreting this as typehint for the values. Do you believe this is an issue on their end? I see that pep-0484 doesn't really apply here, yet I would argue that the same type of type documentation is plausible here.

@has2k1
Copy link
Owner

has2k1 commented Jun 18, 2020

Then, do you know if dict[str, object] would be treated better?

@FFroehlich
Copy link
Author

Then, do you know if dict[str, object] would be treated better?

It is treated the same, so it interprets that the values should be dicts.

@FFroehlich FFroehlich closed this by deleting the head repository Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants