Skip to content

during tokenize, use UTF8 encoding on all platforms#510

Merged
rossbar merged 4 commits intonumpy:mainfrom
drammock:tokenize-encoding
Oct 10, 2023
Merged

during tokenize, use UTF8 encoding on all platforms#510
rossbar merged 4 commits intonumpy:mainfrom
drammock:tokenize-encoding

Conversation

@drammock
Copy link
Contributor

@drammock drammock commented Oct 9, 2023

(This is a PR-as-issue, but if I've guessed the wrong solution please feel free to close or suggest a better fix.)

An MNE-Python user who was trying to build our docs on Windows hit this error today:

Traceback (most recent call last):
  File "C:\Users\Carina\mambaforge\envs\mnedev\Lib\site-packages\sphinx\events.py", line 97, in emit
    results.append(listener.handler(self.app, *args))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Carina\mambaforge\envs\mnedev\Lib\site-packages\numpydoc\numpydoc.py", line 214, in mangle_docstrings
    report = validate(doc)
             ^^^^^^^^^^^^^
  File "C:\Users\Carina\mambaforge\envs\mnedev\Lib\site-packages\numpydoc\validate.py", line 617, in validate
    ignore_validation_comments = extract_ignore_validation_comments(
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Carina\mambaforge\envs\mnedev\Lib\site-packages\numpydoc\validate.py", line 145, in extract_ignore_validation_comments
    for token in tokenize.generate_tokens(file.readline):
  File "C:\Users\Carina\mambaforge\envs\mnedev\Lib\tokenize.py", line 454, in _tokenize
    line = readline()
           ^^^^^^^^^^
  File "C:\Users\Carina\mambaforge\envs\mnedev\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 3922: character maps to <undefined>

re-running after export PYTHONUTF8=1 resolved the issue, so I think explicitly invoking utf-8 during read should also prevent the error without requiring any user action.

Technically I think this is a backwards-incompatible change for windows users who had any non-ASCII characters in their source files if those characters are encoded differently in utf-8 than they are in the system's default codepage (which will vary with OS language settings). However, PYTHONUTF8=1 will become the effective default in 2025 with the release of Python 3.15 (so affected users will need to address this change eventually anyway).

@drammock drammock mentioned this pull request Oct 9, 2023
Closed
@drammock
Copy link
Contributor Author

drammock commented Oct 9, 2023

I've now added a better test that captures the actual case we encountered. ready for review from my end.

drammock
drammock commented Oct 9, 2023
View reviewed changes
@jarrodmillman jarrodmillman mentioned this pull request Oct 9, 2023
Closed
@jarrodmillman
Copy link
Member

jarrodmillman commented Oct 9, 2023

See #505 and #506.

rossbar
rossbar approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Thanks @drammock , the test is very helpful!

@rossbar rossbar merged commit 80a8708 into numpy:main Oct 10, 2023
@rossbar rossbar mentioned this pull request Oct 10, 2023
Closed
@drammock drammock deleted the tokenize-encoding branch December 12, 2023 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jarrodmillman jarrodmillman jarrodmillman approved these changes

@rossbar rossbar rossbar approved these changes

Assignees

No one assigned

Labels

type: Bug fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@drammock @jarrodmillman @rossbar