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

Inconsistent keys on multilingual course cause a cryptic error: "Corresponding RST file names must match in multilingual courses" #134

Open
atilante opened this issue Nov 30, 2021 · 3 comments · May be fixed by #136
Labels
area: config Course configuration area: usability Improve the ease-of-use effort: hours The issue seems to be small and should be doable in hours or few days. requester: Aalto teacher The issue is raised by a teacher from Aalto University type: bug This is a bug which is critical for the correct operation of the system

Comments

@atilante
Copy link
Contributor

atilante commented Nov 30, 2021

Description

If a multilingual course has an exercise or other item that has different
identifier (key) in different language versions, A+ RST Tools produces a
compile error which does not help to solve the problem.

Proposed categories: bug, user interface.

Note: I have a bugfix which could be sent as a pull request.

How to reproduce

Clone the Data Structures and Algorithms Y course repository with the
try to compile the RST material. Exact commands:

git clone git@version.aalto.fi:course/traky.git -b rst-tools-issue-corresponding-names traky-test
cd traky-test
git submodule init
git submodule update
./docker-compile.sh exam

The output of docker-compile.sh script:

CS-A1141 + CS-A1143 exam selected
Compiling course.

--- lines cut ---

build succeeded, 2 warnings.

The HTML pages are in _build/html.
Detected language tree.
Traverse document elements to write configuration index (fi).
Traverse document elements to write configuration index (en).
Joining language tree to one index.

Sphinx error:
Corresponding RST file names must match in multilingual courses:
fi_enrollment
en_enrollment
make: *** [Makefile:62: html] Error 2

Why this is a problem

A+ RST Tools produces the error message:
Corresponding RST file names must match in multilingual courses. While this
is understandable, the lines below the message are rather cryptic:

fi_enrollment
en_enrollment

The user (a course author) should know on which files and which lines in the
RST source code the error is located. Here A+ RST Tools dumps some internal
string representation of some object, and we cannot know whether it is a
file name or something inside an RST file.

In this particular case, the user is left to searching for the error location:

  • Searching the source code with grep -r fi_enrollment returns nothing.
  • Searching with grep -r enrollment returns 97 lines.
  • Renaming files in directory exam-preamble/{en,fi}/*.rst consistently
    does not help.

Investigation

The code location that produces the error message is
https://github.com/apluslms/a-plus-rst-tools/blob/master/lib/toc_languages.py#L316 .

Proposal

I will send a bugfix as a pull request when you have time to review it.

@atilante atilante changed the title Inconsistent keys on multilingual course cause cryptic error: "Corresponding RST file names must match in multilingual courses" Inconsistent keys on multilingual course cause a cryptic error: "Corresponding RST file names must match in multilingual courses" Nov 30, 2021
@markkuriekkinen markkuriekkinen added area: config Course configuration area: usability Improve the ease-of-use effort: hours The issue seems to be small and should be doable in hours or few days. requester: Aalto teacher The issue is raised by a teacher from Aalto University type: bug This is a bug which is critical for the correct operation of the system labels Dec 10, 2021
@markkuriekkinen
Copy link
Contributor

If I understood correctly, you are going to change only the error message. That's great! Please submit the pull request.

@atilante
Copy link
Contributor Author

atilante commented Dec 13, 2021

My pull request would actually contain three things. Here's the commit message summarising those. Issue #134 is number 2 in the following list.

1. Function key_without_language in lib/toc_languages.py had a bug: the
function could only strip language ids of form "id_" and "id-", not "_id" or
"_id_". The bug was discovered with course the CS-A1141 Data Structures and
Algorithms Y.  The function is now reimplemented with a more flexible regular
expression.

2. Function join_keys in lib/toc_languages.py now prints the original
keys in the case they could not be matched. Printing the original key allows
the course material author locate the point of error compared to the stripped
version of the key, which is likely shorter and thus a "needle in a haystack".

3. Unit test file tests/test_toc_languages.py tests the aforementioned
modified functions. The unit tests allow the A-plus RST tools developer to:
(i) easily test the functions of the software without needing to run a
    Sphinx compilation of a test material, which can take minutes;
(ii) document how the functions should respond to certain inputs.

The pull request contains 140 lines, including documentation and unit tests. As this is not a supercritical bugfix, would you like to review it in January rather than before the holidays?

@markkuriekkinen
Copy link
Contributor

That's OK! Since you have already implemented it, you should submit the pull request. I will take a look at it when I'm able, either in December or January. I don't see why you should wait with making the PR. It will be there waiting for me and I can dig in immediately when I have the time.

atilante added a commit to atilante/a-plus-rst-tools that referenced this issue Dec 17, 2021
This commit contains several related modifications to A+ RST Tools issue apluslms#134.
apluslms#134

1. In lib/toc_languages.py, functions key_without_language() and join_keys()
were refactored into methods in the class IndexJoiner. This is more consistent,
as there are other "join"-named methods in IndexJoiner.

2. Method key_without_language() in lib/toc_languages.py had a bug: the
function could only strip language ids of form "id_" and "id-", not "_id" or
"_id_". The bug was discovered with course the CS-A1141 Data Structures and
Algorithms Y.  The function is now reimplemented with a more flexible regular
expression.

3. Method join_keys in lib/toc_languages.py now prints a warning message
into log. The message describes the location of mismatching keys in the
documentation tree, and mismatching keys with their language identifiers.
Example:

    Mismatching keys at modules.1.2.3:
    Language: en Key: sorting_en_something
    Language: fi Key: sorting_fi_different

This should help the course material author locate the error. Moreover,
as the method now logs an error message instead of raising a SphinxError,
the compilation process can show log messages of all mismatching keys at
one compilation run, thus increasing productivity.

4. Unit test file tests/test_toc_languages.py tests the aforementioned
modified functions. The unit tests allow the A-plus RST tools developer to:
(i) easily test the functions of the software without needing to run a
    Sphinx compilation of a test material, which can take minutes;
(ii) document how the functions should respond to certain inputs.
@atilante atilante linked a pull request Dec 17, 2021 that will close this issue
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: config Course configuration area: usability Improve the ease-of-use effort: hours The issue seems to be small and should be doable in hours or few days. requester: Aalto teacher The issue is raised by a teacher from Aalto University type: bug This is a bug which is critical for the correct operation of the system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants