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.

Sign up for GitHub

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

Jump to bottom

Docs: Enable spell check #3882

Closed
wants to merge 28 commits into from
Closed

Docs: Enable spell check #3882

wants to merge 28 commits into from

Conversation

tkknight
Copy link
Contributor

🚀 Pull Request

Description

---- WORK IN IN PROGRESS - DO NOT NOT REVIEW ----

Enable the spell check that was primed in #3779.

Summary of changes:

  • travis linkcheck renamed to checkdocs that will run make linkcheck spelling, instead of just make linkcheck
  • ensured any warnings are raised to errors
  • enabled extension in conf.py
  • updated documented to include make spelling
  • added pip dependency sphinxcontrib-spelling
  • TODO updated file containing allowed words

Consult Iris pull request check list

@tkknight tkknight mentioned this pull request Sep 26, 2020
Closed
2 tasks
@bjlittle bjlittle self-assigned this Sep 27, 2020
@bjlittle
Copy link
Member

@tkknight Let's see if we can squeeze this in....

@bjlittle bjlittle added this to the v3.1.0 milestone Sep 29, 2020
@bjlittle
Copy link
Member

@tkknight This would be lovely to have, but there seems to be some dependency issues 🤔

Thanks for all you're efforts on this, much appreciated, but we're going to leaving this until iris 3.1, hope you don't mind?

@tkknight
Copy link
Contributor Author

The spelling dependency only shows itself when running on travis which is somewhat odd.

make[1]: Entering directory '/home/travis/build/tkknight/iris/docs/iris/src'
sphinx-build -b spelling -W --keep-going . _build
Running Sphinx v3.2.1
[conf.py] Iris Version = 3.0
[conf.py] Iris Release = 3.0.dev0
pyenchant:: looking in system
pyenchant:: with name enchant-2
pyenchant:: with name libenchant-2
pyenchant:: with name enchant
pyenchant:: with name libenchant
pyenchant:: with name enchant-1
pyenchant:: with name libenchant-1
pyenchant:: looking in system
pyenchant:: with name enchant-2
pyenchant:: with name libenchant-2
pyenchant:: with name enchant
pyenchant:: with name libenchant
pyenchant:: with name enchant-1
pyenchant:: with name libenchant-1
pyenchant:: looking in system
pyenchant:: with name enchant-2
pyenchant:: with name libenchant-2
pyenchant:: with name enchant
pyenchant:: with name libenchant
pyenchant:: with name enchant-1
pyenchant:: with name libenchant-1
Extension error:
Could not import extension sphinxcontrib.spelling (exception: The 'enchant' C library was not found and maybe needs to be installed.
See  https://pyenchant.github.io/pyenchant/install.html
for details
)
Makefile:55: recipe for target 'spelling' failed

@tkknight
Copy link
Contributor Author

I am reviewing the state of this. The issue remains in that conda needs an underlying library (enchant) available on the system else it will fail (as does pip). If you have sudo permissions on Linux you can use the commands followed by a conda or pip install of sphinxcontrib-spelling:

sudo apt-get update -y
sudo apt-get update -y enchant

The above commands worked fine on WSL ubunutu 18.04. Adding this sphinxcontrib-spelling dependency works fine on RHEL7 too, however when used on travis-ci (deprecated for this project) or cirrus-ci the enchant dependency is not present and cannot seem to be installed.

An alternative approach will to not include the spelling check as part of the CI tests, but only to run them manually locally (make spelling).

Please let me know if anyone has other suggestions before I action the alternative approach.

@bjlittle
Copy link
Member

@tkknight Okay, after a bit of investigation I've got an easy way forward for this so that it works on cirrus-ci 👍

We can iterate on this and finally get it across the line.

Let me focus on the 3.0.2 release, then come back to bank this in master... does that sound like an acceptable plan?

@bjlittle
Copy link
Member

@tkknight Do you fancy rebasing, then I'll push a PR on your branch with some changes to address this 👍

@bjlittle bjlittle added Peloton 🚴‍♂️ Target a breakaway issue to be caught and closed by the peloton and removed Status: Stalled labels May 25, 2021
@tkknight
Merge branch 'master' into enable_spelling_check
9366e71 
* master: (87 commits)
  Contourf Antialias Fix (SciTools#4150)
  minor tidy of cirrus and nox (SciTools#4152)
  update bug and feature templates (SciTools#4147)
  Update CI environment lockfiles + Cartopy 0.19 changes (SciTools#4125)
  separate arg types from descriptions (SciTools#4100)
  Use assertArrayAllClose for sqrt test (SciTools#4118)
  Removed branch suffix (SciTools#4117)
  Corrected plot_anomaly_log_colouring for new Matplotlib linscale rules. (SciTools#4115)
  replace most recent hashes (SciTools#4112)
  Linkcheck update (SciTools#4104)
  CI lockfiles (SciTools#4108)
  Fix bug in gallery_tests runner (SciTools#4111)
  add logo to conda and pypi badges (SciTools#4088)
  pre-commit-ci update (SciTools#4092)
  Fix cirrus ci and mpl (SciTools#4087)
  Update readme docs stable (SciTools#4089)
  linux kernel version fix for conda 4.10+ (SciTools#4084)
  update docs pypi instructions (SciTools#4077)
  update flake8 pre-commit (SciTools#4067)
  Add GitHub discussions badge (SciTools#4070)
  ...
@bjlittle bjlittle mentioned this pull request Jun 4, 2021
Closed
tkknight and others added 9 commits June 5, 2021 17:41
@tkknight
test
cc34bc6
@tkknight
Merge remote-tracking branch 'upstream/master' into enable_spelling_c…
c5e4274 
…heck

* upstream/master:
  Updated environment lockfiles (SciTools#4182)
  remove __future__ and six references (SciTools#4180)
@tkknight
disabled linkcheck for a quick runn on cirrus...
89317aa
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
b336316 
for more information, see https://pre-commit.ci
@tkknight
testing spelling lang
62753b2
@tkknight
Merge branch 'enable_spelling_check' of github.com:tkknight/iris into…
0ff4ccf 
… enable_spelling_check

* 'enable_spelling_check' of github.com:tkknight/iris:
  [pre-commit.ci] auto fixes from pre-commit.com hooks
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
eed148b 
for more information, see https://pre-commit.ci
@tkknight
enchant debug added
a792831
@tkknight
Merge branch 'enable_spelling_check' of github.com:tkknight/iris into…
c093298 
… enable_spelling_check

* 'enable_spelling_check' of github.com:tkknight/iris:
  [pre-commit.ci] auto fixes from pre-commit.com hooks
@bjlittle bjlittle modified the milestones: v3.1.0, v3.2.0 Aug 4, 2021
@tkknight
Copy link
Contributor Author

Moving to GitHub actions for the documentation CI may resolve this.

@jamesp jamesp modified the milestones: v3.2.0, v3.3.0 Oct 28, 2021
@bjlittle
Copy link
Member

@tkknight Ping... did you want to rebase this PR? Are we still commited to supporting this feature?

Now that we're using GHA, the new platform my play into our favour here.

@tkknight
Copy link
Contributor Author

@tkknight Ping... did you want to rebase this PR? Are we still commited to supporting this feature?

Now that we're using GHA, the new platform my play into our favour here.

I will rebase this PR soonish, there is no rush.

@bjlittle bjlittle mentioned this pull request Jun 20, 2022
Merged
@tkknight
Copy link
Contributor Author

I'm closing this PR now.l The packages to enable this functionality are patchy at best with dependencies having to be manually installed and then may still not work. Lets move on!

@tkknight tkknight closed this Jun 21, 2022
@tkknight tkknight deleted the enable_spelling_check branch November 9, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjlittle bjlittle Awaiting requested review from bjlittle

Assignees

@bjlittle bjlittle

Labels
Peloton 🚴‍♂️ Target a breakaway issue to be caught and closed by the peloton Type: Documentation
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@tkknight @bjlittle @jamesp