Add codespell: workflow, config and fix typos it finds #1226

yarikoptic · 2023-11-13T19:19:29Z

Description

codespell is great to pick up known typos. Unfortunately there were some custom variables used which I had to ignore to not introduce too many changes, but you might like to fix up code to stay aware from the names which are close to the typos.

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would break back-compatibility)
Changes in documentation or new examples for demos and/or use cases.

Branching

All PRs should be made against the dev branch. The master branch is not often merged back to dev.

I realized that too late -- please let me know if I should redo but first want to check if you are interested in such a "change" in general

kushalkolar · 2023-11-13T19:28:19Z

🚀

EricThomson · 2023-11-13T19:30:33Z

This looks very helpful -- maybe should be created from dev not main though. Nice to meet you at the booth will be great to collaborate on dandi and nwb! 💯

pgunn · 2023-11-13T20:27:14Z

Hello,
If we do something like this it will probably be something devs run manually rather than as a GitHub action. It seems to do a good job at finding typos and that's something I like, although it makes a few mistakes too.

For now if you don't mind cutting the tool component and config out and making this a pure typo-fixing diff, and redoing it against dev, that would be a welcome fix and we can haggle our the mistakes it made (and areas where the typo isn't the main problem). Up to you whether you want to close this PR and open a new one, or force push a new diff to your branch and retarget then.

I appreciate the attention to the topic.

yarikoptic · 2023-11-13T21:22:54Z

devs run manually

to run manually you would need a config. Can I keep it? I can remove github workflow if you insist.

NB I personally dislike "manual" things, it "never works" and just piles up problems. Hence we use LOTS of automations and I recommend to adopt some of them. Have a look at our pre-commit configuration for one of the components.

I will now redo/rebase on dev. (if you expect contributions against dev to be primary target, you might like to make dev to be the default branch here, not main. We did similarly change default one to maint for datalad since that is what bugfixes should go against).

yarikoptic · 2023-11-13T21:34:06Z

ok, rebased on dev, redid fixups, removed action (IMHO your loss)

pgunn

Generally looks good; happy to see a lot of typos removed from the codebase. The tool got a few things wrong; left some notes.

It's fine to leave the config stuff in pyproject.

pgunn · 2023-11-14T00:18:24Z

Jenkinsfile

@@ -94,7 +94,7 @@ Check console output at $BUILD_URL to view full results.
 Building $BRANCH_NAME for $CAUSE
 $JOB_DESCRIPTION

-Chages:
+Changes:


Interesting we never caught this before

pgunn · 2023-11-14T00:19:23Z

caiman/base/rois.py

@@ -1095,7 +1095,7 @@ def extract_binary_masks_blob(A,

    Args:
        A: scipy.sparse matrix
-            contains the components as outputed from the CNMF algorithm
+            contains the components as outputted from the CNMF algorithm


We'll have to circle back to this later; the phrasing is weird.

pgunn · 2023-11-14T00:19:56Z

caiman/motion_correction.py

@@ -1642,7 +1642,7 @@ def register_translation(src_image, target_image, upsample_factor=1,
                         "and \"fourier\" values for the ``space`` argument."

    References:
-    .. [1] Manuel Guizar-Sicairos, Samuel T. Thurman, and James R. Fienup,
+    .. [1] Manual Guizar-Sicairos, Samuel T. Thurman, and James R. Fienup,


Can you remove this change?

sure, will force push rewrite, thanks for catching

pgunn · 2023-11-14T00:22:16Z

caiman/source_extraction/cnmf/utilities.py

@@ -1154,9 +1154,9 @@ def fast_graph_Laplacian(mmap_file, dims, max_radius=10, kernel='heat',
        else:
            res = dview.map(fast_graph_Laplacian_pixel, pars, chunksize=128)
        indptr = np.cumsum(np.array([0] + [len(r[0]) for r in res]))
-        indeces = [item for sublist in res for item in sublist[0]]
+        indices = [item for sublist in res for item in sublist[0]]


I feel a little weird that it's correcting code rather than just comments, although this is a change I'd make myself so let's leave it in

pgunn · 2023-11-14T00:22:52Z

caiman/source_extraction/volpy/atm.py

@@ -280,7 +280,7 @@ def denoise_spikes(trace, sampleRate, windowLength, superfactor, threshs=(.4, .6

                tlimit = len(trace)

-                thre = threshold_sets[th]


Let's revert this set of changes though; not its job to weigh in on something that's not a misspelling

FWIW, thre is a frequent typo of three, there, their, the hence was detected. I will make it skip all "thre" but then you would miss the typos.

dropping 6642c7f

pgunn · 2023-11-14T00:26:52Z

demos/notebooks/demo_motion_correction.ipynb

@@ -89,7 +89,7 @@
   "source": [
    "max_shifts = (6, 6)  # maximum allowed rigid shift in pixels (view the movie to get a sense of motion)\n",
    "strides =  (48, 48)  # create a new patch every x pixels for pw-rigid correction\n",
-    "overlaps = (24, 24)  # overlap between pathes (size of patch strides+overlaps)\n",
+    "overlaps = (24, 24)  # overlap between paths (size of patch strides+overlaps)\n",


pgunn · 2023-11-14T00:27:02Z

demos/notebooks/demo_pipeline.ipynb

@@ -131,7 +131,7 @@
    "\n",
    "# motion correction parameters\n",
    "strides = (48, 48)          # start a new patch for pw-rigid motion correction every x pixels\n",
-    "overlaps = (24, 24)         # overlap between pathes (size of patch strides+overlaps)\n",
+    "overlaps = (24, 24)         # overlap between paths (size of patch strides+overlaps)\n",


pgunn · 2023-11-14T00:27:08Z

demos/notebooks/demo_pipeline_cnmfE.ipynb

@@ -120,7 +120,7 @@
    "gSig_filt = (3, 3)       # size of high pass spatial filtering, used in 1p data\n",
    "max_shifts = (5, 5)      # maximum allowed rigid shift\n",
    "strides = (48, 48)       # start a new patch for pw-rigid motion correction every x pixels\n",
-    "overlaps = (24, 24)      # overlap between pathes (size of patch strides+overlaps)\n",
+    "overlaps = (24, 24)      # overlap between paths (size of patch strides+overlaps)\n",


pgunn · 2023-11-14T00:27:14Z

demos/notebooks/demo_pipeline_voltage_imaging.ipynb

@@ -104,7 +104,7 @@
    "                                                # change this one if algorithm does not work\n",
    "max_shifts = (5, 5)                             # maximum allowed rigid shift\n",
    "strides = (48, 48)                              # start a new patch for pw-rigid motion correction every x pixels\n",
-    "overlaps = (24, 24)                             # overlap between pathes (size of patch strides+overlaps)\n",
+    "overlaps = (24, 24)                             # overlap between paths (size of patch strides+overlaps)\n",


pgunn · 2023-11-14T00:27:51Z

docs/CODE_OF_CONDUCT.md

@@ -37,4 +37,4 @@ Second, some behavior is egregious enough that people should not see it in the c

 ## Process

-People who feel that they have been ignored when trying to close a topic or otherwise uncomfortable interactions, or people who feel they have been targeted by egregious behavior that should not be permitted, should contact any of the project organizers. If the claim is not against any of the project organizers, available organizers should confer in the spirit of fairness and due process, investigate as they see fit (including discussing things with the accused), and apply any remedies that they choose. If an organizer is accused, other organizers should confer and apply the same process. Organizers should recuse if they think they cannot be fair.
+People who feel that they have been ignored when trying to close a topic or otherwise uncomfortable interactions, or people who feel they have been targeted by egregious behavior that should not be permitted, should contact any of the project organizers. If the claim is not against any of the project organizers, available organizers should confer in the spirit of fairness and due process, investigate as they see fit (including discussing things with the accused), and apply any remedies that they choose. If an organizer is accused, other organizers should confer and apply the same process. Organizers should recurse if they think they cannot be fair.


recuse was actually the word meant

thanks! ignoring it too

EricThomson · 2023-11-16T22:24:52Z

Note I will probably get to this next week I'm recovering from workshop-sfn 😄 but it does look like it found a lot of useful things.

=== 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 ^^^ note: commit was rebased and adjusted manually for a conflict

=== Do not change lines below === { "chain": [], "cmd": "git-sedi 'between pathes' 'between patches'", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

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

pgunn · 2023-11-17T03:29:13Z

Thanks; this looks good now! Much appreciated!

yarikoptic force-pushed the enh-codespell branch from 94e7c85 to d5cc9f9 Compare November 13, 2023 21:33

yarikoptic changed the base branch from main to dev November 13, 2023 21:33

yarikoptic added 4 commits November 13, 2023 16:33

Add rudimentary codespell config

18459b7

Some custom skips for codespell

17b52d1

Do not git ignore bin/ since it has caiman_gui.py

af14310

skip ptd

87e095c

yarikoptic force-pushed the enh-codespell branch from d5cc9f9 to 69a6e6a Compare November 13, 2023 21:33

pgunn reviewed Nov 14, 2023

View reviewed changes

yarikoptic added 6 commits November 16, 2023 21:26

Ignore name Manuel from codespell

e5c9ad0

Also ignore "thre"

81ccd7e

[DATALAD RUNCMD] Fix pathes typo

d2ac1e4

=== Do not change lines below === { "chain": [], "cmd": "git-sedi 'between pathes' 'between patches'", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^

skip recuse

5504535

[DATALAD RUNCMD] run codespell throughout fixing typo automagically

7b09601

=== 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 69a6e6a to 7b09601 Compare November 17, 2023 02:29

pgunn merged commit 21db43a into flatironinstitute:dev Nov 17, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add codespell: workflow, config and fix typos it finds #1226

Add codespell: workflow, config and fix typos it finds #1226

yarikoptic commented Nov 13, 2023

kushalkolar commented Nov 13, 2023

EricThomson commented Nov 13, 2023

pgunn commented Nov 13, 2023

yarikoptic commented Nov 13, 2023

yarikoptic commented Nov 13, 2023

pgunn left a comment

pgunn Nov 14, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

yarikoptic Nov 17, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

yarikoptic Nov 17, 2023

yarikoptic Nov 17, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

pgunn Nov 14, 2023

yarikoptic Nov 17, 2023

EricThomson commented Nov 16, 2023

pgunn commented Nov 17, 2023

		@@ -280,7 +280,7 @@ def denoise_spikes(trace, sampleRate, windowLength, superfactor, threshs=(.4, .6

		tlimit = len(trace)

		thre = threshold_sets[th]

Add codespell: workflow, config and fix typos it finds #1226

Add codespell: workflow, config and fix typos it finds #1226

Conversation

yarikoptic commented Nov 13, 2023

Description

Type of change

Branching

kushalkolar commented Nov 13, 2023

EricThomson commented Nov 13, 2023

pgunn commented Nov 13, 2023

yarikoptic commented Nov 13, 2023

yarikoptic commented Nov 13, 2023

pgunn 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

EricThomson commented Nov 16, 2023

pgunn commented Nov 17, 2023