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

use TypedDict for return type of legacy detect() #469

Merged
merged 3 commits into from
May 18, 2024

Conversation

pinterior
Copy link
Contributor

@pinterior pinterior commented May 14, 2024

Problem to solve

when I write something like:

x = charset_normalizer.detect(...)["encoding"]

x is inferred as str | float | None instead of actual str | None.

Change

use custom TypedDict type as the return type of detect method if Python version >= 3.8.
(TypedDict is not available in Python 3.7)

Copy link
Member

@Ousret Ousret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. The idea behind this is useful.
There's a stunt we can put in place to avoid the Python 3.7 blocker. Look at my draft suggestion for more.

I suggest otherwise to use the main API wherever you can instead of this old one.

charset_normalizer/legacy.py Outdated Show resolved Hide resolved
charset_normalizer/legacy.py Outdated Show resolved Hide resolved
charset_normalizer/legacy.py Outdated Show resolved Hide resolved
charset_normalizer/legacy.py Outdated Show resolved Hide resolved
@pinterior
Copy link
Contributor Author

There's a stunt we can put in place to avoid the Python 3.7 blocker. Look at my draft suggestion for more.

that makes sense. committed.

@pinterior pinterior requested a review from Ousret May 15, 2024 10:47
@pinterior
Copy link
Contributor Author

CI action "MypyC Tests (3.7, macos-latest) (pull_request)" failing due to actions/runner-images#9770
should I create separate PR to use 'macos-13' image for Python 3.7 instead of 'macos-latest'?

@Ousret
Copy link
Member

Ousret commented May 18, 2024

should I create separate PR to use 'macos-13' image for Python 3.7 instead of 'macos-latest'?

no need, I will reorganize the whole thing anyway.

@Ousret Ousret merged commit 816c998 into jawah:master May 18, 2024
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants