huggingface / huggingface/doc-builder

[course] "Complete Chapter" button fails to become enabled in end-of-chapter quizzes when framework is TF

Open
#428 0 comments 2 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
144
Forks
59
Avg merge
2d 5h
Merged PRs (30d)
13

Description

# Issue description

The button should become enabled once all questions say "You got all the answers!".

Only quizzes with framework selection have this issue (so [chapter1's quiz](https://huggingface.co/learn/nlp-course/chapter1/10) is fine), and only when TensorFlow is selected, as in below for [chapter2](https://huggingface.co/learn/nlp-course/chapter2/8?fw=tf):

https://github.com/huggingface/doc-builder/assets/16626016/d49a1748-b3e2-45fd-8633-85c3331ad4ed

Reproducible at least in Chrome, Edge, and Safari (all on desktop).

## Is this a fatal issue?

No — you can use the sidebar menu to move to any chapter at any time. But it can cause confusion for course takers, making them wonder if there's anything more they need to do on the page when they're actually already done.

# Suspected cause

Looking through the code, it seems likely what is happening is that **questions corresponding to the PyTorch framework mount shortly before `"tf"` is read from the URL query parameter, resulting in some unanswerable questions remaining in the `answers` store in a non-correct state.**

Here is the `answers` store:

https://github.com/huggingface/doc-builder/blob/808766b2b597be24dd51429cc9a9ebb028f44f79/kit/src/lib/stores.ts#L25-L26

And here is where a mounting question updates the store to have an initial, non-correct state for itself:

https://github.com/huggingface/doc-builder/blob/808766b2b597be24dd51429cc9a9ebb028f44f79/kit/src/lib/Question.svelte#L22-L24

Whether questions for PyTorch or questions for TF are mounted is determined by the `fw` value, which is set initially for PyTorch but then overwritten to TF when `"tf"` is extracted from the URL:

https://github.com/huggingface/doc-builder/blob/808766b2b597be24dd51429cc9a9ebb028f44f79/src/doc_builder/convert_md_to_mdx.py#L55-L59

In the time between line 55 and when the `onMount` callback runs to set `fw` to `"tf"` in line 58, we can observe that not only is the `FrameworkSwitchCourse` in the `"pt"` state, but also that questions for PyTorch, like for [#5](https://github.com/huggingface/course/blob/df6be572a4184b48dc3682f085685106fc7a9b08/chapters/en/chapter2/8.mdx#L106-L126), do in fact mount and are shown briefly:

https://github.com/huggingface/doc-builder/assets/16626016/9acef545-1079-49e6-8bcc-81c3ffe80d03

# Proposed Solutions

Note: I'd be happy to submit a PR when I have time! But unfortunately I've had trouble running `doc-builder preview` or `doc-builder build` on my machine to actually try out these fixes, so I'll have to open a separate issue to address that first.

1. A somewhat elegant solution would be possible if the PyTorch questions trigger an `onDestroy` when they are replaced by the TF questions and disappear. I haven't worked with Svelte so I'm not 100% confident that the lifecycle events work as I think they do, but as long as the questions are being unmounted from the DOM instead of just being hidden, then `onDestroy` would be called, no? If true, then right under the [`onMount`](https://github.com/huggingface/doc-builder/blob/808766b2b597be24dd51429cc9a9ebb028f44f79/kit/src/lib/Question.svelte#L22-L24
) in `Question.svelte` we could add an `onDestroy` that removes the key-value pair for the unmounting question from `answers`.

2. If that's not possible, perhaps it would work to simply clear the `answers` store (reset to `{}`) whenever the `fw` value is changed based on the URL query parameters? For instance, by importing the store and resetting it inside the [`onMount`](https://github.com/huggingface/doc-builder/blob/808766b2b597be24dd51429cc9a9ebb028f44f79/src/doc_builder/convert_md_to_mdx.py#L56-L59) in the script from `convert_md_to_mdx`.

3. I suppose the issue can also be solved by eliminating the period of time where `fw` is briefly set to `"pt"`, but this seems more complicated and I'd have to actually run the code locally to get a better idea of what exactly that would entail.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.