Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

Fix and test building documentation is possible without running all notebooks #420

Open
wants to merge 2 commits into
base: v0.x
Choose a base branch
from

Conversation

leezu
Copy link
Contributor

@leezu leezu commented Nov 23, 2018

Description

CI currently only tests that documentation can be built after all notebooks are
run and converted from .md to .ipynb format. Running all notebooks takes some
time. This CI step makes sure, that the documentation can also be built without
converting the notebooks first.

Checklist

Essentials

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Fix dev build of documentation

Comments

Fixes #419

@leezu leezu added bug Something isn't working release focus Progress focus for release labels Nov 23, 2018
@@ -241,7 +241,7 @@ print(sample_tgt_seq)
```

If you'd like to train your own transformer models, you may find the training scripts in our
[model zoo](../../model_zoo/machine_translation/index.rst).
[model zoo](../../../scripts/machine_translation/index.rst).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change we run into readthedocs/recommonmark#93

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it is not valid for the CI build with publication /var/lib/jenkins/workspace/gluon-nlp-docs/docs/examples/machine_translation/transformer.ipynb.rst:link outside of source directory: '../scripts/machine_translation/index.rst'

@leezu leezu force-pushed the fixlocaldocbuild branch 2 times, most recently from 8cd4386 to 9993143 Compare November 23, 2018 15:10
@codecov
Copy link

codecov bot commented Nov 23, 2018

Codecov Report

Merging #420 into master will decrease coverage by 14.64%.
The diff coverage is 40.65%.

@@             Coverage Diff             @@
##           master     #420       +/-   ##
===========================================
- Coverage   84.25%   69.61%   -14.65%     
===========================================
  Files          94      134       +40     
  Lines        8313    11833     +3520     
===========================================
+ Hits         7004     8237     +1233     
- Misses       1309     3596     +2287
Flag Coverage Δ
#PR359 ?
#PR416 ?
#PR420 70.09% <39.63%> (?)
#PR435 70.18% <39.59%> (?)
#PR466 70.74% <39.63%> (?)
#PR503 70.07% <39.61%> (?)
#PR509 70.08% <39.62%> (?)
#PR539 68.52% <39.63%> (?)
#PR541 70.09% <39.63%> (?)
#PR548 70.02% <39.5%> (?)
#PR551 59.06% <35.37%> (?)
#master 70.15% <39.81%> (-13.05%) ⬇️
#notserial 43.16% <27.74%> (-14.88%) ⬇️
#py2 69.32% <40.41%> (-14.61%) ⬇️
#py3 68.72% <40.53%> (-15.32%) ⬇️
#serial 55.73% <26.5%> (-12.26%) ⬇️

@leezu leezu force-pushed the fixlocaldocbuild branch 2 times, most recently from ea751bf to d4c48ef Compare November 23, 2018 15:19
CI currently only tests that documentation can be built after all notebooks are
run and converted from .md to .ipynb format. Running all notebooks takes some
time. This CI step makes sure, that the documentation can also be built without
converting the notebooks first.
@leezu leezu removed the release focus Progress focus for release label Nov 23, 2018
@leezu
Copy link
Contributor Author

leezu commented Dec 20, 2018

I'm not yet sure how to get this to work.

https://github.com/dmlc/gluon-nlp/pull/420/files#diff-be3f0c715a65e953102c0e00f934c72bR244

fixes the issue in the local mode, but breaks on CI. (I didn't look into this again.)

@szha szha changed the base branch from master to v0.x August 13, 2020 02:20
@szha szha requested a review from a team as a code owner August 13, 2020 02:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants