[DocTests Speech] Add doc tests for all speech models #15031

patrickvonplaten · 2022-01-04T14:32:26Z

What does this PR do?

This PR revives the doc tests and adds doc tests for all speech models now that the new docs are finished.

Before submitting

This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
Did you read the contributor guideline,
Pull Request section?
Was this discussed/approved via a Github issue or the forum? Please add a link
to it if that's the case.
Did you make sure to update the documentation with your changes? Here are the
documentation guidelines, and
here are tips on formatting docstrings.
Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

.github/workflows/doctests.yml

src/transformers/file_utils.py

LysandreJik

Great! Let's take the opportunity to have the doctests passing on this PR before merging! (5 files failing now, but for simple issues I believe)

.github/workflows/doctests.yml

src/transformers/models/wavlm/modeling_wavlm.py

sgugger

Thanks a lot for tackling those! Could you develop what issue you had with black exactly?

.github/workflows/doctests.yml

src/transformers/file_utils.py

utils/documentation_tests.txt

…into start_cleaning_doc_tests

sgugger · 2022-01-25T18:47:18Z

utils/prepare_for_doc_test.py

+    diff = clean_code != code
+    if diff:
+        print(f"Overwriting content of {code_file}.")
+        with open(code_file, "w", encoding="utf-8", newline="\n") as f:


I think we want to put them in some temp dir to run the tests and not overwrite the existing one (otherwise we will get modifications we don't want when running the doctest locally).

sounds good!

Actually I made two commands now - one that adds the line - one that reverts it. Think this is more intuitive for local testing instead of copying the files and this way we also don't need to add a temp path before every file to be tested

We should document somewhere the instruction that has to be run before/after testing locally then.

Adding a long intro in the beginning of the file. @sgugger - do you think it could be a good idea to add a README.md to utils so that community contributors have some docs on how to use the utils scripts?

I was thinking more of making a section in the doc page for the tests.

Ok that would be good for me as well!

sgugger

Thanks, great to have those running again!

sgugger · 2022-01-26T16:57:40Z

.github/workflows/doctests.yml

+      - "tests/**"
+      - ".github/**"
+      - "templates/**"
+    types: [assigned, opened, synchronize, reopened]


I think we decided to remove assigned from those @LysandreJik, can you confirm?

Yeah but Patrick mentioned that he'll remove that from the PR before merging as it only needs to run on schedule

sgugger · 2022-01-26T16:58:52Z

src/transformers/models/unispeech/modeling_unispeech.py

+        ...     "hf-internal-testing/tiny-random-unispeech-sat"
+        ... )
+        >>> model = UniSpeechForPreTraining.from_pretrained("microsoft/unispeech-large-1500h-cv")
+        >>> # TODO: Add full pretraining example


I would remove the doctest syntax while waiting for a tested example, but I don't think it's good to remove the example from the doc entirely.

Yeah, the problem is that it was just copy-pasted and never worked.

sgugger · 2022-01-26T17:01:27Z

src/transformers/models/unispeech_sat/modeling_unispeech_sat.py

-        >>> # for contrastive loss training model should be put into train mode
-        >>> model.train()
-        >>> loss = model(input_values, mask_time_indices=mask_time_indices).loss
+        >>> # TODO: Add full pretraining example


Yeah, the problem is that it was just copy-pasted and never worked.

sgugger · 2022-01-26T17:02:29Z

.github/workflows/doctests.yml

+      - name: Clean files after doctests
+        run: |
+          python utils/prepare_for_doc_test.py src docs --remove_new_line


This won't be run if the previous instruction fails (but not sure we care)

True, but yeah I'm also not sure if it matters really as it's the only test run in this suite

sgugger · 2022-01-26T17:03:09Z

utils/prepare_for_doc_test.py

+    diff = clean_code != code
+    if diff:
+        print(f"Overwriting content of {code_file}.")
+        with open(code_file, "w", encoding="utf-8", newline="\n") as f:


We should document somewhere the instruction that has to be run before/after testing locally then.

LysandreJik

This looks good to me, thank you for working on it @patrickvonplaten

LysandreJik · 2022-01-26T17:17:12Z

.github/workflows/doctests.yml

+      - "tests/**"
+      - ".github/**"
+      - "templates/**"
+    types: [assigned, opened, synchronize, reopened]


Yeah but Patrick mentioned that he'll remove that from the PR before merging as it only needs to run on schedule

utils/prepare_for_doc_test.py

patrickvonplaten · 2022-01-27T12:19:15Z

.github/workflows/doctests.yml

@@ -35,8 +35,16 @@ jobs:
        run: |
          apt -y update && apt install -y libsndfile1-dev
          pip install --upgrade pip
-          pip install .[dev]


will revert this once we have a stable dev docker image

patrickvonplaten · 2022-01-27T12:19:25Z

.github/workflows/doctests.yml

@@ -19,7 +19,7 @@ env:

 jobs:
  run_doctests:
-    runs-on: [self-hosted, docker-gpu, single-gpu]
+    runs-on: [self-hosted, docker-gpu-test, single-gpu]


leaving on -test for now

patrickvonplaten added 20 commits May 19, 2021 09:07

fix_torch_device_generate_test

f7197df

remove @

5f70018

Merge branch 'master' of https://github.com/huggingface/transformers

69c3b8c

Merge branch 'master' of https://github.com/huggingface/transformers

8420743

Merge branch 'master' of https://github.com/huggingface/transformers

c0ea119

Merge branch 'master' of https://github.com/huggingface/transformers

fe63aeb

Merge branch 'master' of https://github.com/huggingface/transformers

56bdd4b

Merge branch 'master' of https://github.com/huggingface/transformers

553cd9b

Merge branch 'master' of https://github.com/huggingface/transformers

d15a897

KMerge branch 'master' of https://github.com/huggingface/transformers

8e638c8

Merge branch 'master' of https://github.com/huggingface/transformers

a8cec49

Merge branch 'master' of https://github.com/huggingface/transformers

29271f1

Merge branch 'master' of https://github.com/huggingface/transformers

0fd83bf

Merge branch 'master' of https://github.com/huggingface/transformers

9da740a

Merge branch 'master' of https://github.com/huggingface/transformers

aa86e48

Merge branch 'master' of https://github.com/huggingface/transformers

86a56a7

doc tests

eb8a1db

up

a944f41

up

30cf078

fix doctests

08a63cf

patrickvonplaten commented Jan 4, 2022

View reviewed changes

.github/workflows/doctests.yml Outdated Show resolved Hide resolved

patrickvonplaten commented Jan 4, 2022

View reviewed changes

.github/workflows/doctests.yml Show resolved Hide resolved

patrickvonplaten commented Jan 4, 2022

View reviewed changes

src/transformers/file_utils.py Outdated Show resolved Hide resolved

patrickvonplaten commented Jan 4, 2022

View reviewed changes

src/transformers/file_utils.py Show resolved Hide resolved

patrickvonplaten changed the title ~~[DocTests] Revive doc tests~~ [WIP][DocTests] Revive doc tests Jan 4, 2022

LysandreJik approved these changes Jan 5, 2022

View reviewed changes

.github/workflows/doctests.yml Show resolved Hide resolved

src/transformers/models/wavlm/modeling_wavlm.py Outdated Show resolved Hide resolved

LysandreJik self-requested a review January 5, 2022 08:27

ydshieh mentioned this pull request Jan 7, 2022

update the file list for doc testing #15010

Closed

sgugger approved these changes Jan 10, 2022

View reviewed changes

Merge branch 'master' of https://github.com/huggingface/transformers …

3a4785c

…into start_cleaning_doc_tests

patrickvonplaten changed the title ~~[WIP][DocTests] Revive doc tests~~ [WIP][DocTests] Add doc tests for all speech models Jan 25, 2022

patrickvonplaten changed the title ~~[WIP][DocTests] Add doc tests for all speech models~~ [WIP][DocTests Speech] Add doc tests for all speech models Jan 25, 2022

patrickvonplaten added 10 commits January 25, 2022 19:09

save intermediate

c751453

add more logic

eeb1f50

new change

42af7a8

improve

8a0a386

next try

ba693af

next try

2457aae

next try

eea7c96

next try

c012390

fix final spaces

3be3806

fix final spaces

c72ee1c

sgugger reviewed Jan 25, 2022

View reviewed changes

patrickvonplaten added 7 commits January 26, 2022 12:39

improve

1523332

renaming

b50415a

correct more bugs

ec84c69

finish wavlm

68f279f

add comment

fcef9ef

run on test runner

786ddb7

finish all speech models

6feeb1a

patrickvonplaten changed the title ~~[WIP][DocTests Speech] Add doc tests for all speech models~~ [DocTests Speech] Add doc tests for all speech models Jan 26, 2022

sgugger approved these changes Jan 26, 2022

View reviewed changes

LysandreJik approved these changes Jan 26, 2022

View reviewed changes

patrickvonplaten added 3 commits January 27, 2022 13:12

adapt

2a23517

fix

f3219a3

finish

f8a11d7

patrickvonplaten commented Jan 27, 2022

View reviewed changes

patrickvonplaten merged commit 9f831bd into huggingface:master Jan 27, 2022

patrickvonplaten deleted the start_cleaning_doc_tests branch January 27, 2022 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DocTests Speech] Add doc tests for all speech models #15031

[DocTests Speech] Add doc tests for all speech models #15031

patrickvonplaten commented Jan 4, 2022 •

edited

Loading

LysandreJik left a comment

sgugger left a comment

sgugger Jan 25, 2022

patrickvonplaten Jan 25, 2022

patrickvonplaten Jan 26, 2022

sgugger Jan 26, 2022

patrickvonplaten Jan 27, 2022

sgugger Jan 27, 2022

patrickvonplaten Jan 27, 2022

sgugger left a comment

sgugger Jan 26, 2022

LysandreJik Jan 26, 2022

sgugger Jan 26, 2022

patrickvonplaten Jan 27, 2022

sgugger Jan 26, 2022

patrickvonplaten Jan 27, 2022

sgugger Jan 26, 2022

patrickvonplaten Jan 27, 2022

sgugger Jan 26, 2022

LysandreJik left a comment

LysandreJik Jan 26, 2022

patrickvonplaten Jan 27, 2022

patrickvonplaten Jan 27, 2022

[DocTests Speech] Add doc tests for all speech models #15031

[DocTests Speech] Add doc tests for all speech models #15031

Conversation

patrickvonplaten commented Jan 4, 2022 • edited Loading

What does this PR do?

Before submitting

Who can review?

LysandreJik left a comment

Choose a reason for hiding this comment

sgugger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgugger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LysandreJik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickvonplaten commented Jan 4, 2022 •

edited

Loading