fix(report): write HTML export as utf-8#705
Open
immu4989 wants to merge 1 commit into
Open
Conversation
export_and_save_report() called open(output_file, "w") without an explicit encoding, so the file was written with locale.getpreferredencoding(False). On Windows that's cp1252, which raises UnicodeEncodeError whenever the nbconvert HTML body contains non-Latin1 characters (feature names with accents, user-supplied text in additional data, etc.). Add encoding="utf-8" to the open() call so the export is consistent across platforms. Add a regression test that mocks HTMLExporter + the local open binding to assert the encoding kwarg is passed, since monkeypatching locale doesn't affect Python 3.12's internal default-encoding resolution. Fixes MAIF#699
Collaborator
|
Hi @immu4989, We have a wider ongoing work on the report generation to get rid of papermill/jupyter dependencies. I let your PR opened in the meantime but it will likely not be merged. |
Contributor
Author
|
Thanks @guerinclement, that makes total sense , and good to hear #699 will be addressed as part of the wider rework. Happy to leave the PR open as a one line stopgap in case a Windows user reports the bug again before the new report module lands. Feel free to cherry pick the Looking forward to seeing the refactor land. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
export_and_save_report()writes the rendered HTML report viaopen(output_file, "w")without an explicitencoding=, so thefile is written with
locale.getpreferredencoding(False). On Windowsthat's
cp1252, which raisesUnicodeEncodeError: 'charmap' codec can't encode character ...whenever the nbconvert HTML body contains non-Latin1 characters
(feature names with accents, user-supplied text in additional data,
dataset string values, etc.).
The reporter (@dalestee) diagnosed the issue and prescribed the fix in
the issue body. This PR applies that fix exactly: add
encoding="utf-8"to theopen()call atshapash/report/generation.py:93.Grepped the rest of
shapash/for sibling sites — every otherwrite-side
open()call is in binary mode and every read-side onetouches ASCII-only bundled JSON, so this is the only line that needs
to change.
Fixes #699
Type of change
How Has This Been Tested?
Added
test_export_and_save_report_writes_utf8intests/integration_tests/test_report_generation.py. It mocksHTMLExporterand the localopenbinding inshapash.report.generation, callsexport_and_save_report, andasserts that every write-mode
open()call carriesencoding="utf-8".The test uses
mock.patchrather than monkeypatchinglocale.getpreferredencoding,because Python 3.12 resolves
open()'s default encoding at a lowerlevel than the
localemodule exposes — monkeypatchinglocalenolonger changes the actual behavior. A contract test on the call
signature is the most reliable way to catch a future regression on
CI runners that default to UTF-8.
pytest tests/integration_tests/test_report_generation.py -q→ 8 passed (7 pre-existing + 1 new)ruff format --check shapash/report/generation.py tests/integration_tests/test_report_generation.py→ cleanTest Configuration:
Checklist: