classification
Title: Deprecate suspicious.py?
Type: Stage: patch review
Components: Documentation Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: mdk Nosy List: mdk, ned.deily, rhettinger
Priority: normal Keywords: patch

Created on 2020-11-01 23:49 by mdk, last changed 2020-12-18 09:48 by mdk.

Pull Requests
URL Status Linked Edit
PR 23313 merged mdk, 2020-11-16 08:08
PR 23802 merged mdk, 2020-12-16 14:20
Messages (9)
msg380167 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-11-01 23:49
I was not here 21 years ago when it was introduced [1], but according to the commit message it was introduced to find leftover Latex mardown.

It tries to find 4 patterns in Sphinx node text (not in raw rst files):

::(?=[^=])|            # two :: (but NOT ::=)

This one has ~100 false positive in susp-ignored.csv (pypi classifiers, slices, ipv6, ...) 

:[a-zA-Z][a-zA-Z0-9]+| # :foo

This one has ~300 false positive in susp-ignored.csv (slices, C:\, ipv6, ...)


`|                     # ` (seldom used by itself)

This one has ~20 false positive in susp-ignored.csv (mostly reStructuredText in code-blocks)

(?<!\.)\.\.[ \t]*\w+:  # .. foo: (but NOT ... else:)

This one does not have false positives.

The script, on my laptop (with a core i9), is slow (4mn20s), and it's probably way slower on the CI.

I tried to search for `suspicious is:pr in:comments` on github to see if it's usefull:

- 2 contributor had an issue with the script (gh-9748, gh-21940)
- 5 had to add false positive to susp-ignored.csv (gh-20556, gh-13772, gh-11481, gh-9317, gh-6915)
- 4 had to update susp-ignored.csv (gh-11769, gh-5552, gh-3694, gh-2719)
- 1 did not addedd to susp-ignored but changed to avoid a false positive (gh-18939)

Case where it actually helped:

- Finding an error: (gh-12562 .. literalinclude: instead of .. literalinclude::)
- Finding refs in code block (gh-7413)
- Writing plaintext in Misc/NEWS (gh-1339)

I'd go for enhancing rstlint.py (which is fast, ~1s on my laptop) a bit to try to handle the `.. literalinclude:` missing a `:` errors, and dropping suspicious.

So I'd appreciate feedback on this script, did it helped you recently?

1: https://github.com/python/cpython/commit/700cf28f410521066f40671f1da7db0302d753fd
msg380169 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-11-02 00:07
I would love to see this disappear.  For me, it has been a recurring time waster.
msg380171 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2020-11-02 00:51
If it still does useful checks, we could just limit it to running by release managers during the release manufacturing process; it is already run then. It would still allow for problems to be caught and fixed by the RM prior to release tagging.  But I don't have a strong opinion about its overall usefulness. I recall it catching at least one problem in a release I was involved with and that was probably before we had the CI checks.
msg381075 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-11-16 07:50
Good idea Ned!

So proposed plan:

- Drop it from docs build and the CI to avoid time loss.
- Add it as a step of PEP 101, for a few release, for good measures
- I'll check it from time to time between releases, just to ensure it does not accumulate tons of things to do on the the release day.

If during a few release, we notice this tool is no loger usefull we'll be able to drop it.

If we spot some errors that can be migrated to the rstlint.py we'll do.

If this tool is in fact usefull, we'll have to think about it again.
msg381802 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-11-25 09:18
New changeset c9c6e9f89aa68ce8094393a1a5575b67d26bc8c8 by Julien Palard in branch 'master':
bpo-42238: Doc: Remove make suspicious from the CI and docs builds. (GH-23313)
https://github.com/python/cpython/commit/c9c6e9f89aa68ce8094393a1a5575b67d26bc8c8
msg382219 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-12-01 08:21
I'll try to track this closely, the drop of `make suspicious` has been merged in master, but not backported yet, one week ago.

Up to today, no commit has introduced suspicious warnings (0 false positive, 0 false negative).

I think I'll soon backport the PR, if no one objects, because the current situation is a bit risky: some PR could get merged to master but fail the tests during the backport.
msg383149 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-12-16 11:06
Today I spotted one error that suspicious could have spotted:

    -both ``Callable``s no longer validate their ``argtypes``, in 
    +both ``Callable``\ s no longer validate their ``argtypes``, in 

I'm fixing it in https://github.com/python/cpython/pull/23800 

`make suspicious` found it as:

    [whatsnew/changelog:23] "`" found in "Callable``s no longer validate their ``argtypes"

So I'll doodle around this to see if tools/rstlint.py or Sphinx itself can see the error.
msg383171 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-12-16 14:29
I created https://github.com/python/cpython/pull/23802 with a port of the "wrong backtick detector" (at least, one aspect from it) from suspicious to rstlint.

I dropped the suspicious check three weeks ago, and found a single committed issue since then, which was fixable (and is fixed) in rstlint, so I'll continue this experience and see where it goes.
msg383292 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2020-12-18 09:48
New changeset b9735420aa8ecd2555fe3dc000863bc50487334b by Julien Palard in branch 'master':
bpo-42238: Check Misc/NEWS.d/next/ for reStructuredText issues. (GH-23802)
https://github.com/python/cpython/commit/b9735420aa8ecd2555fe3dc000863bc50487334b
History
Date User Action Args
2020-12-18 09:48:14mdksetmessages: + msg383292
2020-12-16 14:29:48mdksetmessages: + msg383171
2020-12-16 14:20:05mdksetpull_requests: + pull_request22662
2020-12-16 11:06:41mdksetmessages: + msg383149
2020-12-01 08:21:25mdksetmessages: + msg382219
2020-11-25 09:18:14mdksetmessages: + msg381802
2020-11-16 08:08:04mdksetkeywords: + patch
stage: patch review
pull_requests: + pull_request22204
2020-11-16 07:50:08mdksetmessages: + msg381075
2020-11-02 00:51:19ned.deilysetnosy: + ned.deily
messages: + msg380171
2020-11-02 00:07:26rhettingersetnosy: + rhettinger
messages: + msg380169
2020-11-01 23:49:23mdkcreate