[SPARK-31155] Remove pydocstyle tests

[nchammas]

What changes were proposed in this pull request?

As discovered here #27910 (comment), pydocstyle tests were not running anywhere (not on Jenkins; not on GitHub).

This PR enables those tests.

It also seems like a large hill to climb to enable any meaningful checks, so we're going to just rip pydocstyle out for now.

Why are the changes needed?

Presumably, we defined those doc style tests because we care about whatever it is they enforce. Since we're not actually testing anything, though, it's better to clear the cruft.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Will check the GitHub workflow logs on this PR.

[nchammas]

pydocstyle tests are now running.

[SparkQA]

Test build #119794 has finished for PR 27912 at commit 3f09453.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #119795 has finished for PR 27912 at commit 0e71162.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

@nchammas, my current thought is that:

• We should exclude the pinning versions at dev/requirements.txt
• I am less strongly against about pinning versions at Github Actions.
• It currently ignores so many rules.
  • Can you try to enable some rules? I am sure some rules will be able to enable as of fe75ff8.
  • If there are still many rules to disable, should we maybe just revert pydocstyle itself at this moment?

[nchammas]

Here's a count of the errors when I try enabling all the rules:

+------------+-----+
|  error_code|count|
+------------+-----+
|        D100|  483|
|        D101|  327|
|        D102| 2040|
|        D103|  321|
|        D104|   23|
|        D105|  244|
|        D106|   14|
|        D107|  330|
|        D200| 2622|
|        D201|    4|
|        D202|  110|
|        D203|  931|
|        D204|  212|
|        D205| 1767|
|        D207|    6|
|        D208|   30|
|        D209|   19|
|        D210|  164|
|        D211|  116|
|        D212| 5129|
|        D213|  617|
|        D214|    3|
|        D300|   14|
|        D301|   49|
|        D400| 2119|
|        D401| 2743|
|        D402|  304|
|        D403|   80|
|        D404|   34|
|        D405|    4|
|        D406|    7|
|        D407|   11|
|        D412|    4|
|        D413|   11|
|        D415| 2117|
|        D416|    2|
|        D417|    2|
+------------+-----+

So it looks like there are a handful of rules we could perhaps enable, like D206, D215, D302, and D408-D411.

[nchammas]

I'd say the high-value rules are the D100s, which cover missing docstrings in public objects, though I wonder what "public" means to pydocstyle: Anything not named with a leading underscore?

We're doing pretty badly in the missing docstring category, so if we aren't interested in making a push to clean up in that area perhaps it would be better to just rip pydocstyle out all together.

[SparkQA]

Test build #119808 has finished for PR 27912 at commit eed7164.

• This patch fails due to an unknown error code, -9.
• This patch merges cleanly.
• This patch adds no public classes.

[nchammas]

Retest this please.

[SparkQA]

Test build #119815 has finished for PR 27912 at commit eed7164.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

Thanks @nchammas! Shall we revert SPARK-23367 in this PR for now? It should be fine to convert SPARK-31155 to describe the revert of SPARK-23367. cc @holdenk FYI.

[srowen]

OK by me either way.

[SparkQA]

Test build #119876 has finished for PR 27912 at commit 18de9d7.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #119885 has finished for PR 27912 at commit 34ec6a3.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

Jenkins retest this please

[BryanCutler]

Seems ok with me to remove given it's not being used and ignores many useful checks. I would support enabling again if there was an effort to fix some of checks.

[SparkQA]

Test build #119894 has finished for PR 27912 at commit 34ec6a3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

Merged to master and branch-3.0.

[shaneknapp]

late "lgtm", thanks coronavirus!

…

On Mon, Mar 16, 2020 at 6:42 PM Hyukjin Kwon ***@***.***> wrote: Closed #27912 <#27912> via b4748ca <b4748ca> . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27912 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMIHLENGFASA7ZQZKDCSKTRH3IO5ANCNFSM4LIS6HGA> .