ENH: Support translated notebooks#600
Conversation
|
I won't include this in the current patch release, as I don't have enough bandwidth to avoid blocking the Jupyter Book release that needs to follow. I'll circle back around to this PR soon :) |
|
@OriolAbril - Adding a test here would be really useful. I would a translated version of a test notebook should be sufficient. |
for more information, see https://pre-commit.ci
|
I did my best to modify one of the tests in |
|
It seems different versions of sphinx generate different translation related metadata and the xml comparison fails. I don't know how to fix that |
I have some open PRs that revisit the test requirements, as well as planning to do another cleanup of the versions in GHA. One those are done and this still runs into CI issue, we can revisit with some version dependent conditionals, or just state that translations support will require a newer sphinx than the stated minimum supported one. |
|
The way the PR introduces to handle translations works in all versions. However, the html/xml metadata related to translations differs for the different versions. The xml generated with sphinx 8 which is the one I added to the tests folder is the following: In sphinx<8 the If it were possible to ignore this metadata when comparing against the xml output then tests would pass for all versions, and would still check that the actual translated content is injected together. |
for more information, see https://pre-commit.ci
|
@OriolAbril - Experimenting and combing the code a little bit through I'm fairly certain that the logic is the opposite: we need the old sphinx-generated file for comparison and then ignore the extra bit of metadata in conftest. There are already a couple of similar cases. As I was iterating on this locally, I've pushed the changes directly to your branch. I think this is good to go now, but I'll leave this open so @agoose77 can double-check it, too. |
|
Thanks @bsipocz! |
|
I don't see any objections, so let's go ahead with the merge here. Thanks @OriolAbril for your patience. |
Closes #587. The issue points to the relevant sphinx source,
where it can be seen sphinx "marks" translated strings with
<translated>at theend of the
source_pathargument, which is then stored as thecurrent_sourceattribute ofthe
docutils.nodes.documentclass, which is the second argument ofParser.parse.This adds an extra condition to fall back to myst-parser which allows using myst-nb with translated
content.
It would be nice to add some test for this but I am not familiar enough with the test suite yet,
I'll try to add one in the coming days. It can probably reuse one of the existing notebooks,
but it will be necessary to add a
localefolder with the translations, will it be ok to addit directly in the
testsfolder?So far I have only checked it works locally on a couple repos I had
(both executing and not executing notebooks).