CI: Enable Spellcheck in dockbuild
State: closed

This is the initial PR to try and fix the issue #21354 the sphinx spelling extension needs every name to be divided and added to each line in order to get the name marked as spelt correctly. The file `names_wordlist.txt` is updated on every run of the function `get_authors()` located inside`scripts/announce.py`.

Since the issue #21396 is related to my previous PR I attempted to resolve this issue here as well, I followed the suggestion of @jorisvandenbossche and added a try/except to the `conf.py` if the dependency doesn't exist then nothing will happen.

@datapythonista since you submitted a PR(#21397) to try and fix the issue, perhaps you have an opinion about this bit of code. Also, if you would like to do the changes on your own PR I will just delete this bit and use your solution 😄 👍 

I would like some feedback on this first attempt, the generator `[names.extend(re.sub('\W+', ' ', x).split()) for x in cur.union(pre)]` seems a bit smelly but does the trick, should this be refactored to use a long form and make it more readable? I extended the list to names because split() was creating a list inside the main list and this makes every name to be split and added to the same list (this is probably very memory inefficient though)

I tried to run the script from my machine but I got an issue with it, probably I'm doing something wrong but when I run `./scripts/announce.py $GITHUB v0.15.1..v0.23.0` I get the error message:

```
usage: announce.py [-h] [--repo REPO] revision_range 
announce.py: error: unrecognized arguments: v0.15.1..v0.23.0
```

Not sure what I'm doing wrong, to be honest. If I run just the `get_authors()` function everything seems to be fine (including the v0.15.1..v0.23.0 format) and the names_wordlist is updated successfully.

I will look forward to read your opinions about this PR 👍 
----
- [ ] closes #21396 #21354
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Docs

Comments:
Hello @FabioRosado! Thanks for updating the PR.







Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on June 24, 2018 at 14:32 Hours UTC
Heya yeah that is my plan but I have some busy days at work I plan to work on this tomorrow or Wednesday. 

I appreciate your help @jorisvandenbossche I need to check datapythonista’s PR and see if he changed anything and then I’ll check if I need to add something or not. 
Also, good point on the `ImportError` I didn’t thought about that. 

I was planning to add some sort of message saying why it failed but I wasn’t sure if I should add a print or some sort of logging (I just need to check what is used in pandas)
@FabioRosado, I can work on the simple PR to make the documentation build work again later this afternoon. Then you can check for a better solution when you have time.
@datapythonista  that sounds awesome I didn't want to step in on your PR that's why I added that bit as a thing that could be done in case you did it differently haha my schedule just changed tomorrow so I will have to work on this on Wednesday :/
No worries. Just updated the PR so the documentation can be built when the dependencies for the spellcheck are not present, as this is kind of urgent. Then, with no rush and whenever you have time, you can work on a good approach to manage the dependencies for the spellcheck.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=h1) Report
> Merging [#21402](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f1aa08c4631aea435d66c161557e44f7c72a618e?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/21402/graphs/tree.svg?height=150&src=pr&token=eZ4WkYLtcO&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master   #21402   +/-   ##
=======================================
  Coverage    91.9%    91.9%           
=======================================
  Files         153      153           
  Lines       49547    49547           
=======================================
  Hits        45537    45537           
  Misses       4010     4010
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.3% <ø> (ø)` | :arrow_up: |
| #single | `41.78% <ø> (ø)` | :arrow_up: |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=footer). Last update [f1aa08c...1290adf](https://codecov.io/gh/pandas-dev/pandas/pull/21402?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

On this update, I have decided to create another function in `scripts/announce.py` that uses the `get_authors` in order to get all the names since version 0.3.0. I wanted to use the names already on the wordlist (because it contains some names located in `release.rst` that aren't in `get_authors`) so I turned those names into a set, then updated the set with the names that I got from `get_authors`. 

The plan to use sets is to try and avoid duplicated names. I have also updated the regex to avoid some characters but not all non-words (names such as O'Leary as marked as a single word).

Now when the command `python make.py spellcheck` is run the function `update_name_wordlist()` will be called before the spelling extension - so all names will be updated before spellcheck is run.

I also updated the bit that datapythonista wrote to just add `ModuleNotFoundError` and replaced logging.warn by logging.warning - because the .warn method is deprecated.

I'm not sure if the code in `update_name_wordlist` is the most efficient one so I'm more than happy to change something if you think there are better ways to do this.

Finally, I have been reading about CI and how to add it to the docbuild but I am unsure how to do this, I have also read the scripts located in the ci folder. I'd appreciate some guidance in this matter as I have no idea how to proceed from here (my apologies).

Finally, is it worth it to make a test in order to try and bump coverage up since it went down a bit?

Thank you!

I added the code down there because I thought I needed to let the whole script run and then do the spell check at the end. I have moved that bit closer to `./make.py` so this should run once the docs are build
Yes we do need the list, like I said on my previous PR, the Sphinx extension reads the words to be whitelisted from a txt file and currently there is no way to use stdout to whitelist words :smile:
The doc build on Travis is failing with "ModuleNotFoundError: No module named 'git'" (check the last build to see the log output of the doc build)
@jorisvandenbossche The build that failed is that #21511 or the build from my previous commit? If it was the latter I was getting a problem from the lint script, but removing the `ModuleNotFoundError` from the try/except (when trying to import the spelling library) seem to fix the issue.

But now that you mention that, i'm importing the function from announce.py which uses `gitpython`
and `pygithub`, I guess this could potentially be a problem, I'll try to import these dependencies along with sphinx spelling and just log if they weren't found.

-- EDIT --
I have tested my code on python 3.6 and I am currently having issues with importing the function `update_names_wordlist()` from the announce.py file. On 3.5 there is no issue so I will need to try and figure out how to update the list when the spellcheck runs.

Ideally, I would like the list to be updated when users run the command `python make.py spellcheck` and not just from travis since the build can break locally but not on travis. 
> The build that failed is that #21511 or the build from my previous commit?

No, it is from this PR. Each PR (and each commit that you add to a PR) is tested on travis. You can follow the link to that by clicking on the "All checks have passed" -> "Show all checks". 

Note that it is not actually failing (travis will show it as green), but you can see in the doc build log that it is actually failing. Eg for the last commit added here: https://travis-ci.org/pandas-dev/pandas/jobs/393561877

The reason this is failing is simply because the correct dependencies are not installed on travis (so the installation script needs to be updated, need to check where you can add this)

> I'll try to import these dependencies along with sphinx spelling and just log if they weren't found.

Yes, that's a good idea. 
Although that makes yet another optional dependency for a dev environment ...
BTW, I think a very valuable contribution would be to add a feature to sphinxcontrib-spelling to skip a file ... (that would solve many problems here!)
> BTW, I think a very valuable contribution would be to add a feature to sphinxcontrib-spelling to skip a file ... (that would solve many problems here!)

And it might be as simple as skipping based on the `docname` in https://github.com/sphinx-contrib/spelling/blob/fa8c4b6140d7be0dcaa2aefbf690b84090bb0884/sphinxcontrib/spelling/builder.py#L144
@jorisvandenbossche  Yeah you are right that would make things so much easier, I'll try to submit a PR to the spelling extension to add the possibility to avoid certain files. 

I guess this PR will have to either be placed on hold or closed since if this gets merged then the build will eventually fail when new names are added - unless we add another dependency to make this work on the build.

I will wait until I hear from you as to what is the best course of action with these issues 👍 
> 'll try to submit a PR to the spelling extension to add the possibility to avoid certain files.

That would be really cool!

> I guess this PR will have to either be placed on hold or closed

I think we can go ahead with this for now, as long as the script is there to update the list of names to not have the build fail. We can later remove that again if the feature to ignore certain files is added.
I had to remove ModuleNotFoundError since linting kept marking this as non-existent as this except is only available on Python 3.6, I have updated the name list back to all the names as well.

I did leave the reference to the function that updates the wordlist on the spellcheck function, but if you run the announce.py script the list will also be updated (since its not called from `main`). Hope this is okay.

Will look forward to fixing some issue here once I figure out how to avoid files/folders in the spelling extension haha
can you rebase / update
I thought this PR wasn't accepted as there was some different opinions about it
rereading i think this is ok if we can just auto generate the list of authors from the commit history rather than hard coding everything 
@jreback @jorisvandenbossche are you happy merging this if @FabioRosado rebases?

My opinion is that the whole typo validation adds too much complexity to the project. And that being based in enchant, which is a deprecated project, it even makes less sense to have it. We can surely have a separate repo in the pandas org that checks in the pandas repo whether there is any typo with all this system we have, that would be very useful. But having it in pandas, when afaik none of the maintainers was able to set it up, doesn't seem to be worth.

In any case, can you let @FabioRosado if he should rebase so we can merge it, or just close the PR. Thanks!
> My opinion is that the whole typo validation adds too much complexity to the project. And that being based in enchant, which is a deprecated project, it even makes less sense to have it. We can surely have a separate repo in the pandas org that checks in the pandas repo whether there is any typo with all this system we have, that would be very useful. But having it in pandas, when afaik none of the maintainers was able to set it up, doesn't seem to be worth.

ok let's close this then.