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: UnicodeDecodeError #490

Merged
merged 3 commits into from
Nov 30, 2020
Merged

fix: UnicodeDecodeError #490

merged 3 commits into from
Nov 30, 2020

Conversation

sbrugman
Copy link
Contributor

nbQA helps us to keep notebooks to the same standards as the rest of the code. If you're serious about your code standards, you should keep them consistent across both notebooks and python scripts. Great addition to the ecosystem, thanks!

Version nbQA 0.5.1 throws an UnicodeDecodeError exception on my Windows machine. The proposed changes in this PR resolve it for my configuration, however I could imagine it's desirable to configure/automatically detect the charset for more general usage.

Find the updated configuration of how we use nbQA in pandas-profiling here: ydataai/ydata-profiling#628

sbrugman and others added 3 commits November 29, 2020 21:56
Fixes utf-8 encoding

```
- hook id: nbqa-black
- exit code: 1
- files were modified by this hook

Traceback (most recent call last):
  File "c:\users\cees closed\.cache\pre-commit\repo9j8ifsr9\py_env-python3\lib\site-packages\nbqa\__main__.py", line 579, in _run_on_one_root_dir
    replace_source.main(
  File "c:\users\cees closed\.cache\pre-commit\repo9j8ifsr9\py_env-python3\lib\site-packages\nbqa\replace_source.py", line 88, in main
    notebook_json = json.loads(notebook.read_text())
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.8_3.8.1776.0_x64__qbz5n2kfra8p0\lib\pathlib.py", line 1236, in read_text
    return f.read()
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.8_3.8.1776.0_x64__qbz5n2kfra8p0\lib\encodings\cp1252.py", line 23, in decode
    return codecs.charmap_decode(input,self.errors,decoding_table)[0]
UnicodeDecodeError: 'charmap' codec can't decode byte 0x81 in position 1184027: character maps to <undefined>
```
fix: utf-8 encoding
@codecov-io
Copy link

codecov-io commented Nov 29, 2020

Codecov Report

Merging #490 (a8d6177) into master (b7bcb95) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #490   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          731       731           
=========================================
  Hits           731       731           
Impacted Files Coverage Δ
nbqa/replace_source.py 100.00% <100.00%> (ø)
nbqa/save_source.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7bcb95...a8d6177. Read the comment docs.

@s-weigand s-weigand added bug Something isn't working enhancement New feature or request labels Nov 30, 2020
Copy link
Contributor

@s-weigand s-weigand left a comment

Choose a reason for hiding this comment

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

LGTM Thanks for the PR

Don't know how many times I had this problem on windows, where python used the codepage from the console encoding and assumed that files use the same encoding.

import this

...
Explicit is better than implicit.
...

@MarcoGorelli
Copy link
Collaborator

Thanks @sbrugman , both for your kind words (mind if we include them as a testimonial?) and for this fix

@MarcoGorelli MarcoGorelli merged commit 0e72cfc into nbQA-dev:master Nov 30, 2020
@MarcoGorelli
Copy link
Collaborator

@all-contributors add @sbrugman for bugs

@allcontributors
Copy link
Contributor

@MarcoGorelli

I've put up a pull request to add @sbrugman! 🎉

@sbrugman
Copy link
Contributor Author

mind if we include them as a testimonial?

Not at all, feel free @MarcoGorelli

@sbrugman
Copy link
Contributor Author

Thanks for the quick merge

@MarcoGorelli
Copy link
Collaborator

cool, thanks! Have made a patch release (0.5.2) with this fix

@sbrugman sbrugman deleted the patch-1 branch November 30, 2020 19:11
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b7bcb95) to head (a8d6177).
Report is 227 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #490   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          731       731           
=========================================
  Hits           731       731           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants