Add codespell support (config, workflow to detect/not fix) and make it fix few typos #26315

yarikoptic · 2024-04-19T14:22:13Z

More about codespell: https://github.com/codespell-project/codespell .

I personally introduced it to dozens if not hundreds of projects already and so far only positive feedback.

CI workflow has 'permissions' set only to 'read' so also should be safe.

There were numerous commits where codespell was used "manually" to fixup some of the typos.
CI: use codespell, flake8 to check code before merging? #13694 still contemplates idea of running codespell on PRs
An early attempt to introduce codespell CI was abandoned: BLD: Codespell for NumPy CI #19594 -- IMHO now is better time!
I did lapack_lite last in the separate commits if there is desire to not touch it (although I saw commits fixing typos in it as well).
I did some variable renames to take them away from "close to a typo" form, might be worth having a look and suggesting alternative names.
I did fix older changelogs as I think there is no point to cherish typos in them, but they could also be ignored.

charris · 2024-04-19T14:28:05Z

Please don't touch the changelogs, they match the title used in the corresponding PR, typos and all.

ngoldbaum · 2024-04-19T15:58:25Z

doc/source/dev/governance/governance.rst

@@ -262,7 +262,7 @@ conflict of interests include, but are not limited to:
 All members of the Council shall disclose to the rest of the Council any
 conflict of interest they may have. Members with a conflict of interest
 in a particular issue may participate in Council discussions on that
-issue, but must recuse themselves from voting on the issue.
+issue, but must recurse themselves from voting on the issue.


not a big fan of a adding automated spell-checking if we'll have to ignore it making mistakes like this...

rright -- I even ran into this particular one before -- recuse is indeed rare and I failed to spot it in review, sorry about that. I will whitelist it.

yarikoptic · 2024-04-19T18:03:29Z

Thanks for quick feedback @charris and @ngoldbaum !

I

rebased (apparently there were new typos in main)
ran with current master of codespell to ensure picking up as many typos as possible (thought that typos detected by CI were due to mine being outdated but it just needed rebase)
whitelisted some more per feedback above (do not touch changelogs, recuse is ok so we might recuse into folders at some point as a side-effect ;-) )
adjusted some patches to make lint satisfied (one change with spaces around = was for "old" code, but triggered to be checked since I changed the line)

doc/changelog/1.25.0-changelog.rst

mattip

On the one hand, this found many typos. On the other, it has definite opinions about variable names in tests, not all are valid. This is kind of like black in that it operates on the entire code base, and like black seems to be over-strict.

mattip · 2024-05-30T08:06:45Z

numpy/_core/memmap.py

-    >>> newfp = np.memmap(filename, dtype='float32', mode='r', shape=(3,4))
-    >>> newfp
+    >>> fp_new = np.memmap(filename, dtype='float32', mode='r', shape=(3,4))
+    >>> fp_new


Did codespell do this? It seems unnecessary

nope, I did that in 639693e :

commit 639693ee4fa82173f5444e76cfec47704d655ea6 Author: Yaroslav Halchenko <debian@onerussian.com> Date: Fri Apr 19 10:02:05 2024 -0400 DOC: adjust variables in example to be fp_{kind} instead of {k}fp and fp{k} I think it makes them more consistent (always a suffix) and also more readable since more descriptive of the taken action. This is triggered to avoid "looks-like-a-typo" hits in codespell for fpr and likely others

mattip · 2024-05-30T08:06:56Z

numpy/_core/memmap.py

-    >>> fpr = np.memmap(filename, dtype='float32', mode='r', shape=(3,4))
-    >>> fpr.flags.writeable
+    >>> fp_ro = np.memmap(filename, dtype='float32', mode='r', shape=(3,4))
+    >>> fp_ro.flags.writeable


see above... too close to for typo etc.

mattip · 2024-05-30T08:07:56Z

numpy/_core/tests/test_einsum.py

@@ -816,7 +816,7 @@ def test_einsum_fixedstridebug(self):
        tp = np.tensordot(A,  B,  axes=(0,  0))
        assert_equal(es,  tp)
        # The following is the original test case from the bug report,
-        # made repeatable by changing random arrays to aranges.
+        # made repeatable by changing random arrays to arranges.


revert this

good catch, adding arange and aranges to whitelist

mattip · 2024-05-30T08:08:28Z

numpy/_core/tests/test_indexing.py

-        arro = np.zeros((4, 4))
-        arr = arro[::-1, ::-1]
+        arr0 = np.zeros((4, 4))
+        arr = arr0[::-1, ::-1]


see above and specifically commit 8afd1bf

mattip · 2024-05-30T08:08:46Z

numpy/_core/tests/test_multiarray.py

-        padd = np.repeat(np.min(arr), 513)
-        rarr = np.concatenate((arr, padd))
+        arr_ = np.repeat(np.min(arr), 513)
+        rarr = np.concatenate((arr, arr_))


mattip · 2024-05-30T08:09:12Z

numpy/_core/tests/test_simd.py

-            broadcasti = self.setall(i)
-            assert broadcasti == [i] * self.nlanes
+            broadcast_i = self.setall(i)
+            assert broadcast_i == [i] * self.nlanes


mattip · 2024-05-30T08:10:14Z

numpy/_pyinstaller/hook-numpy.py

@@ -18,7 +18,7 @@
    # communal Conda bin directory. DLLs from NumPy's dependencies must also be
    # collected to capture MKL, OpenBlas, OpenMP, etc.
    from PyInstaller.utils.hooks import conda_support
-    datas = conda_support.collect_dynamic_libs("numpy", dependencies=True)
+    _ = conda_support.collect_dynamic_libs("numpy", dependencies=True)


No need to have a left-side at all

mattip · 2024-05-30T08:10:41Z

numpy/lib/_shape_base_impl.py

@@ -37,7 +37,7 @@ def _make_along_axis_idx(arr_shape, indices, axis):
    shape_ones = (1,) * indices.ndim
    dest_dims = list(range(axis)) + [None] + list(range(axis+1, indices.ndim))

-    # build a fancy index, consisting of orthogonal aranges, with the
+    # build a fancy index, consisting of orthogonal arranges, with the


mattip · 2024-05-30T08:11:55Z

pyproject.toml

+# Skip all shortish (up to 5) ACRONYMS
+ignore-regex = '"worl"|https://citeseerx\.ist\.psu\.edu|\b([A-Z][a-z]{1,9}|[A-Z]{1,5})\b|.*codespell-ignore.*|/seh/'
+# some common hits and some "questionable" and false positives (rare words)
+ignore-words-list = 'nd,nin,numers,numer,inout,ccompiler,fo,CCompiler,dout,ans,ser,ot,asign,siz,alow,fro,delimitor,alo,dum,desig,recuse,implementor'


Can this be folded into a multiline statement?

Ha -- it seems to can. Splitting into multiple lines

mattip · 2024-05-30T08:12:31Z

doc/source/release/1.14.0-notes.rst

@@ -334,7 +334,7 @@ eliminating their use internally and two new C-API functions,

 have been added together with a complementary flag,
 ``NPY_ARRAY_WRITEBACKIFCOPY``. Using the new functionality also requires that
-some flags be changed when new arrays are created, to wit:
+some flags be changed when new arrays are created:


Please leave these files out.

skipping and dropping manual 21d95b2

yarikoptic · 2024-05-30T13:55:38Z

commented on above review by @mattip , dropped a commit, rebased the latter ones, reran codespell (so now no changes to release notes), force pushing now.

yarikoptic · 2024-05-30T13:58:09Z

uff, main got new typos since previous time, rebasing/rerunning again!

Used codespellit list-hits-sorted | sed -n -e '/6 /,9999p' | tac | f2 | tr '\n' ',' to find and sort

…espell + parms -> params

… fp{k} I think it makes them more consistent (always a suffix) and also more readable since more descriptive of the taken action. This is triggered to avoid "looks-like-a-typo" hits in codespell for fpr and likely others

…tic load there)

=== Do not change lines below === { "chain": [], "cmd": "codespell -w -i 3 -C 2", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

… names

… automagically but ignoring the failure due to ambigous typos === Do not change lines below === { "chain": [], "cmd": "codespell -w || :", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

…us typos === Do not change lines below === { "chain": [], "cmd": "codespell -w -i 3 -C 2", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

Funny thing is that it does not catch it on 'main' for some reason but catches here... it is likely because python tools/linter.py --branch origin/main works on the diff only (too fast on main) and thus whenever those spaces were introduced, it did not check and never flipped since then. I changed that line, and it was checked and flagged.

=== Do not change lines below === { "chain": [], "cmd": "codespell -w", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

ngoldbaum reviewed Apr 19, 2024

View reviewed changes

yarikoptic force-pushed the enh-codespell branch from 14d5762 to 66941b7 Compare April 19, 2024 17:59

yarikoptic commented Apr 19, 2024

View reviewed changes

doc/changelog/1.25.0-changelog.rst Outdated Show resolved Hide resolved

yarikoptic force-pushed the enh-codespell branch from 66941b7 to bb7d7f3 Compare April 19, 2024 18:07

mattip reviewed May 30, 2024

View reviewed changes

yarikoptic force-pushed the enh-codespell branch from bb7d7f3 to 31e1ea2 Compare May 30, 2024 13:55

yarikoptic added 20 commits May 30, 2024 09:58

Add github action to codespell main on push and PRs

13e46f1

Add rudimentary codespell config

6f3dec6

For now exclude lapack_lit from spelling

9fc1893

Add some common hits to be ignored by codespell

c5d3b37

Used codespellit list-hits-sorted | sed -n -e '/6 /,9999p' | tac | f2 | tr '\n' ',' to find and sort

Ignore names and acronyms

cb27afb

Rename "datas" to "data_samples"

fe6641c

more skips

0cae3d9

tune some variables or annotate to be skipped to avoid triggering cod…

8b1baab

…espell + parms -> params

Annotate line with secnd - I think on purpose

d88a54b

Replace typo-close doub with 'd' in an example

1bd9709

Unclear typo fix guess

88b76b5

Replace typo-close padd with arr_ (name padd seems to not carry seman…

11b95f0

…tic load there)

Mark intentional typo to be skipped

708da13

[DATALAD RUNCMD] Do interactive fixing of leftover ambigous typos

8261289

=== Do not change lines below === { "chain": [], "cmd": "codespell -w -i 3 -C 2", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

Re-enable codespelling lapack_lite and skip few common short variable…

202b72c

… names

[DATALAD RUNCMD] Do interactive fixing of lapack_lite leftover ambigo…

cce396c

…us typos === Do not change lines below === { "chain": [], "cmd": "codespell -w -i 3 -C 2", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

ignore changelogs and "recuse"

b341edd

replace typo-close "notin" with "not_in" variable

62c3904

yarikoptic added 9 commits May 30, 2024 09:58

fix ambiguous "fals -> false" typo

5b96b27

Whitelist implementor for codespell

116c205

Use _ for unused return value (makes line shorter to pass lint)

23bba46

Add arange and aranges to whitelist

500abb9

Place skips for codespell into multiple lines for easier digestion

54136af

Do not codespell ./doc/source/release/*-notes.rst

0614587

do not codespell recarray_from_file.fits - just raises warnings

76bb931

[DATALAD RUNCMD] run codespell throughout fixing typos automagically

15e0565

=== Do not change lines below === { "chain": [], "cmd": "codespell -w", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

yarikoptic force-pushed the enh-codespell branch from 31e1ea2 to 15e0565 Compare May 30, 2024 14:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #26315

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #26315

yarikoptic commented Apr 19, 2024

charris commented Apr 19, 2024

ngoldbaum Apr 19, 2024

yarikoptic Apr 19, 2024

yarikoptic commented Apr 19, 2024

mattip left a comment

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

mattip May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

mattip May 30, 2024

yarikoptic May 30, 2024

yarikoptic commented May 30, 2024

yarikoptic commented May 30, 2024

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #26315

Are you sure you want to change the base?

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #26315

Conversation

yarikoptic commented Apr 19, 2024

charris commented Apr 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarikoptic commented Apr 19, 2024

mattip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarikoptic commented May 30, 2024

yarikoptic commented May 30, 2024