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

1948 Add Specifying Encoder Rule #2082

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

GibranHL0
Copy link
Contributor

@GibranHL0 GibranHL0 commented Jun 29, 2021

I have made things!

Closes #1948

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

🙏 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

@GibranHL0
Copy link
Contributor Author

I used the following approach:

The open function has two ways to have the parameter encoding by the keyword encoding='utf-8' or being the fourth argument open('file.txt', 'r', -1, 'utf-8') . Then, I applied a visit_Call to check if the call is the function open and if it has the keyword encoding or a fourth parameter in the arguments.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Great work! Some minor things to refactor and we are ready to go!

wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
wemake_python_styleguide/visitors/ast/attributes.py Outdated Show resolved Hide resolved
@sobolevn
Copy link
Member

sobolevn commented Jun 29, 2021

For some reason our Actions do not start 🤔

@sobolevn sobolevn mentioned this pull request Jun 29, 2021
.. versionadded:: 0.16.0
"""

error_template = 'Found unespecified encoding'
Copy link
Member

Choose a reason for hiding this comment

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

Found `open()` call without specified encoding

@sobolevn
Copy link
Member

Maybe we should recreate this PR? Actions might ignore it for some reason.

@GibranHL0
Copy link
Contributor Author

Sure, I will open a new PR with the changes you made.

@GibranHL0
Copy link
Contributor Author

I opened a new PR #2084 with the changes in the UnspecifiedEncodingViolation and the encoders to suppress warnings.

@sobolevn sobolevn mentioned this pull request Jun 30, 2021
4 tasks
@sobolevn
Copy link
Member

Thank you, I will close new one, let's keep more history here.

@GibranHL0
Copy link
Contributor Author

Sure. Also, I committed a new change to the conf.py file because the encoder was not correct and it was causing warnings.

@Sxderp
Copy link
Contributor

Sxderp commented Oct 11, 2021

Encoding doesn't make sense when opened in binary mode. As the added tests don't have any binary mode tests I think this may be an oversight. What I'm not sure is if it should be OK to go without specifying encoding when in binary mode or if encoding=None is better. However, I don't know how difficult it is to make an exception for binary mode.

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.

Require specifying encoding when using "open" (re: PEP597)
3 participants