DOC: Fix encoding of docstring validation for Windows
State: closed

In Windows, the `validate_docstrings.py` script fails because an encoding error. It has been fixed here.

PR done in the London python sprints meetup.

CC: @datapythonista 

Labels:
name: Docs
name: CI
description: Continuous Integration
name: Windows
description: Windows OS

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=h1) Report
> Merging [#25466](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/fe1654faa86836a0007bb513504e57c5c9935b8b?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #25466      +/-   ##
==========================================
+ Coverage   91.74%   91.74%   +<.01%     
==========================================
  Files         173      173              
  Lines       52923    52923              
==========================================
+ Hits        48554    48555       +1     
+ Misses       4369     4368       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.31% <ø> (ø)` | :arrow_up: |
| #single | `41.73% <ø> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/25466/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `87.66% <0%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25466?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/25466?src=pr&el=footer). Last update [fe1654f...639302e](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=h1) Report
> Merging [#25466](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/94c8c94dccfa7934699c314420f0898d81425519?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #25466      +/-   ##
==========================================
- Coverage   91.98%   91.98%   -0.01%     
==========================================
  Files         175      175              
  Lines       52374    52374              
==========================================
- Hits        48178    48174       -4     
- Misses       4196     4200       +4
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.53% <ø> (ø)` | :arrow_up: |
| #single | `40.72% <ø> (-0.15%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/25466/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/25466/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.9% <0%> (-0.12%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25466?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/25466?src=pr&el=footer). Last update [94c8c94...5edb1e8](https://codecov.io/gh/pandas-dev/pandas/pull/25466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

running
```
.\scripts\validate_docstrings.py --errors=GL08 --format=json
```
returns
```
UnicodeEncodeError: 'charmap' codec can't encode character '\u2155' in position 254: character maps to <undefined>
```
Sure but we need automated tests. You can place one in `scripts/tests/test_validate_docstrings.py`
@kpapdac can you merge master to resolve build failure and add a test for this?
yes, i'm on it!
ok, i gave it a go, i've never done this before but my best guest is to add to the ```scripts/tests/test_validate_docstrings.py``` in TestDocstringClass class the following
```
    @pytest.mark.parametrize('name', 'pandas.Series.str.isdecimal')
    def test_exit_status_for_write_validation_results_to_json(self, name):
        msg = 'UnicodeEncodeError in "{}"'.format(name)
        with pytest.raises(UnicodeEncodeError, match=msg):
            validate_docstrings.Docstring.validate_pep8(name)
```
For this specific function the test should fail as there is an encoding issue. I doubt that it works though as it passes the test without making any change to ```scripts\validate_docstrings.py```. I have no idea how to move on with this, any help appreciated!
Hey @kpapdac sorry if I'm misunderstanding, but shouldn't your written test pass if you make no change to the `scripts\validate_docstrings.py` since it raises an UnicodeEncodeError on Windows? I think your test should be checking if UnicodeEncodeError does not get raised when calling the `validate_pep8` function on Windows.
Hi @RjLi13 thank you for having a look at this and apologies for the delay. I think you're right. I changed the test to
```
    def missing_encoding_write_to_file(self):
        """
        Examples
        --------
        >>> try:
        ...   docstr = validate_docstrings.Docstring('pandas.Series.str.isdecimal')
        ...   result = docstr.validate_pep8()
        ...   next(result)
        ...   print(1)
        ... except:
        ...   0
        1
        """
        pass
```
so as to fail when the exception is raised. Now, when I change ```scripts\validate_docstrings.py``` as per the pull request, the test passes the ```file.write(content.encode('utf-8'))``` part but somehow fails on ```application.run_checks([file.name])```. It gives me an Valueerror ```write to closed file```. Do you have any idea what's going wrong here? I appreciate your help!
@kpapdac Push what you have so we can take a look at the code.
can you merge master
@jreback I did merge master 3 days ago, there should be no conflict now.
@RjLi13 I also pushed my changes in test_validate_docstrings.py
@jreback just to clarify I merged and pushed encode_error branch to my master and got a successful built from Travis. Is there anything else i should do to resolve this? Thanks
@kpapdac your comment above suggests you did but I don't see a test as part of this. Can you double check you included that?
hi @WillAyd, I added a test for the encoding issue in test_validate_docstrings.py in my master. The name of the test is test_encode_content_write_to_file. 
Also apologies, I created a new pull request for this by mistake, so please ignore. 
I'm a new contributor so please let me know if something doesn't make sense.
@kpapdac you'll need to add that test to this PR. Make it locally on your `encode_error` branch then do the following:

```sh
git fetch upstream
git merge upstream/master
git push origin encode_error
```

It should update this PR with your test
Hello @kpapdac! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-05-06 19:50:57 UTC
thanks @WillAyd , does it now look ok?
thank you @datapythonista, @WillAyd for all the help
@kpapdac can you address comments from @datapythonista ? Should be able to get this in thereafter
Thanks @kpapdac !
Thank you!