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

Distinguish between final and intermediate ResultDict #266

Merged
merged 1 commit into from Jul 13, 2022
Merged

Distinguish between final and intermediate ResultDict #266

merged 1 commit into from Jul 13, 2022

Conversation

jdufresne
Copy link
Contributor

The IntermediateResultDict has optional fields where as the final, all
fields can't be None.

Carried over from the typeshed types:
https://github.com/python/typeshed/blob/ee09a67c5cf61effaae1bdb894571eac4fc9ac6b/stubs/chardet/chardet/__init__.pyi

@jdufresne
Copy link
Contributor Author

CC @hauntsaninja

@dan-blanchard
Copy link
Member

I'm not enough of a mypy user to understand the point of this one. Why do we care about then difference between these two?

The IntermediateResultDict has optional fields where as the final, all
fields can't be None.

Carried over from the typeshed types:
https://github.com/python/typeshed/blob/ee09a67c5cf61effaae1bdb894571eac4fc9ac6b/stubs/chardet/chardet/__init__.pyi
@jdufresne
Copy link
Contributor Author

jdufresne commented Jul 13, 2022

This is something @hauntsaninja suggested in the typeshed thread: python/typeshed#8182

It was carried over from the typeshed implementation.

I'm not 100% sure on its utility and if it can't be justified as useful, I'm fine to see this closed without merge.

@dan-blanchard dan-blanchard merged commit 023e7ea into chardet:main Jul 13, 2022
@hauntsaninja
Copy link

Thanks for the PR and the review!

The idea here is that the following should pass type check:

result = chardet.detect(byte_str)
print("Language is: " + result["language"])

Without this PR, type checkers would complain because it looks like there's the possibility of trying to do "Language is: " + None.

@jdufresne jdufresne deleted the final-result branch July 13, 2022 20:39
dan-blanchard added a commit that referenced this pull request Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants