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 CharsetMatches type error #813

Merged

Conversation

junholee6a
Copy link
Contributor

@junholee6a junholee6a commented May 8, 2023

from_bytes() has return type CharsetMatches, and CharsetMatches.best() has return type CharsetMatch|None. Notice that CharsetMatches and CharsetMatch are distinct. Also, CharsetMatches.first() is equivalent to calling CharsetMatches.best(). Fixes the code trying to call CharsetMatch.first(), which is undefined. This also fixes the following mypy type errors (from #811 (comment)):
Screenshot 2023-05-08 at 5 58 15 PM

Note, the existing code should've thrown an error from trying to call undefined method CharsetMatch.first(), so this portion of the code is probably not covered by unit tests. It wouldn't be easy to write a test for this since this part of the code is only reached if the first encoding detector fails.

junholee6a added 2 commits May 9, 2023 00:18
from_bytes() has return type CharsetMatches, and CharsetMatches.best()
has return type CharsetMatch|None. Notice that CharsetMatches and
CharsetMatch are distinct. Also, CharsetMatches.first() is equivalent
to calling CharsetMatches.best(). Fixes the code trying to call
CharsetMatch.first(), which is undefined. This in turn fixes mypy type
errors.

Note, the previous code should've thrown an error but didn't, so this
portion of the code is probably not covered by unit tests. It wouldn't
be easy to write a test for this since this part of the code is only
reached if the first encoding detector fails.
@junholee6a junholee6a force-pushed the fix-charsetmatch-selection branch from 06bdfa5 to 4f53fc1 Compare May 9, 2023 04:18
@taylorfturner taylorfturner enabled auto-merge (squash) May 9, 2023 13:40
@taylorfturner taylorfturner added the static_typing mypy static typing issues label May 9, 2023
@taylorfturner taylorfturner merged commit d2356c5 into capitalone:main May 9, 2023
@junholee6a junholee6a deleted the fix-charsetmatch-selection branch May 9, 2023 20:13
@taylorfturner
Copy link
Contributor

#747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
static_typing mypy static typing issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants