Skip to content

chardet is not required for anything #304

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

Merged
merged 1 commit into from
Aug 9, 2024
Merged

chardet is not required for anything #304

merged 1 commit into from
Aug 9, 2024

Conversation

tonylampada
Copy link
Collaborator

@tonylampada tonylampada commented Aug 7, 2024

Description

Removes chardet dependency.

  • not used by our code
  • doesn't show up as a dependency of anything in pipdeptree
  • tests pass without it
  • command line still seems to work
  • git history does not provide a clear reason for why it was added

Type of change

  • Maintainability

How has this change been tested, please provide a testcase or example of how you tested the change?

  • Tests passing
  • Used the CLI locally

Any specific deployment considerations

No need to release because of this

@tonylampada tonylampada marked this pull request as ready for review August 7, 2024 13:51
@tonylampada tonylampada self-assigned this Aug 7, 2024
@tonylampada tonylampada requested a review from capjamesg August 7, 2024 13:51
@tonylampada tonylampada marked this pull request as draft August 7, 2024 14:01
@tonylampada
Copy link
Collaborator Author

oh wait...

Some explanation about how chardet is used.
https://github.com/psf/requests/blob/79b74ef704dc0d804937c0d015c5e9c3b02b79bd/docs/user/advanced.rst#encodings
4:51
When you receive a response, Requests makes a guess at the encoding to use for decoding the response when you access the :attr:Response.text <requests.Response.text> attribute. Requests will first check for an encoding in the HTTP header, and if none is present, will use charset_normalizer or chardet to attempt to guess the encoding.

@tonylampada
Copy link
Collaborator Author

Yeah, response.text seems to work ok without it too
CleanShot 2024-08-07 at 11 05 35@2x

@tonylampada tonylampada marked this pull request as ready for review August 7, 2024 14:29
@tonylampada tonylampada merged commit b1c63d6 into main Aug 9, 2024
8 checks passed
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.

2 participants