Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

codespell: config, github action (to detect new typos), and 2 typos fixed #65

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

yarikoptic
Copy link
Contributor

No description provided.

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

codecov-commenter commented Nov 1, 2023

Codecov Report

Merging #65 (a968574) into main (1cd834f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #65   +/-   ##
=======================================
  Coverage   66.80%   66.80%           
=======================================
  Files          10       10           
  Lines         946      946           
  Branches      198      198           
=======================================
  Hits          632      632           
  Misses        291      291           
  Partials       23       23           
Files Coverage Δ
src/pystow/cache.py 67.08% <ø> (ø)
src/pystow/utils.py 61.91% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cthoyt
Copy link
Owner

cthoyt commented Nov 6, 2023

Hi @yarikoptic, thanks for the contribution. However, I am not comfortable adding a new CI element that I'm not familiar with. can you point to a way to run this spell checker locally?

@cthoyt cthoyt mentioned this pull request Nov 15, 2024
cthoyt added a commit that referenced this pull request Nov 15, 2024
This PR isolates the typo fixes proposed in #65. I'm still not
comfortable to add a new CI script without an explanation of how to run
it locally, so this just takes the concrete changes from that PR.

---------

Co-authored-by: Yaroslav Halchenko <[email protected]>
@cthoyt
Copy link
Owner

cthoyt commented Nov 15, 2024

I upstreamed the typo fixes from this PR in #75 then tried to switch from using the proposed GitHub Action to something explicit in my tox configuration. Now, they are in the Tests / Lint action.

However, I'm getting some false positives. How should I go about addressing these?

@yarikoptic
Copy link
Contributor Author

sorry, missed initial comment! FWIW I like the strategy of encoding running codespell as part of dev process. Frequently we place it into pre-commit as well, so kudos on keeping succinct CI setup!

as for false positives, depends on the kind. Checkout https://github.com/codespell-project/codespell/blob/main/README.rst#ignoring-words , and you have that ignore-words already in the config, and since then there is capability for inline ignores. So "choose your poison" ;)

[tool.codespell]
skip = '.git,*.pdf,*.svg'
#
# ignore-words-list = ''
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# ignore-words-list = ''
# ignore-words-list = ''
ignore-regex = "\bassertIn\b"

For case sensitive matching IIRC (adding whole on mobile)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just wondering if this will always be something I have to chase. How does it deal with function names, which might be out of my control if they don't match?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is such a common fear ;) in the projects using CamelCase or some other mixedCase, I just add regex to catch those, smth like (pick your poison)

❯ grep ignore-regex.*A-Z */.codespellrc
alphafold/.codespellrc:ignore-regex = (^\s*"image/\S+": ".*|\b[A-Z]+\b)
citation-file-format/.codespellrc:ignore-regex = ["`][A-Z][A-Z]["`]
gdcm-gdcm/.codespellrc:ignore-regex = \b([A-Z]+|[A-Z][a-z]*)\b
hpc-toolkit/.codespellrc:ignore-regex = HETATM -e CONECT|\([A-Z]\)[a-z]+
hugo/.codespellrc:ignore-regex = .*TE\?MP.*|.*codespell-ignore.*|[A-Z]?[a-z]*[A-Z][a-zA-Z]*|/extras/crossreferences/|Januar 2023|\\ndoes|.*ä.*
lazydocker/.codespellrc:ignore-regex = (\b[A-Za-z][a-z]*[A-Z]\S+\b|\.edn\b|\S+…|\\nd\b)
lazygit/.codespellrc:ignore-regex = (\b[A-Za-z][a-z]*[A-Z]\S+\b|\.edn\b|\S+…|\\nd\b)
NeuroML-Documentation/.codespellrc:ignore-regex = ^\s*"image/\S+": ".*|\b[a-z]*[A-Z][a-zA-Z]*\b
qlever/.codespellrc:ignore-regex = \b([A-Z]*[a-z]+[A-Z][a-zA-Z]*)\b|.*(Lorem ipsum|eleifend|feugait|codespell-ignore).*
quarto/.codespellrc:ignore-regex = \b[a-z]+[A-Z][A-Za-z]*\b
remark/.codespellrc:ignore-regex = \b(Hart|[A-Z]+)\b|.{300,}.*
saunafs/.codespellrc:ignore-regex = \b[A-Z]*[a-z]+[A-Z][A-Za-z]*\b|/mnt/[a-z0-9]+|.*\bcodespell-ignore\b.*
scidata/.codespellrc:ignore-regex = .*[úáüéè].*|\b[A-Z]+\b|\bvalue=".."
webknossos/.codespellrc:ignore-regex = \b(Manuel|[a-z]+[A-Z][a-zA-Z]*|H Mattern|Nat Commun|couldn&apos;t)\b

but as you can see here it was just a single hit, so I personally would not sweat for now. ;-) generally it is better to avoid names (variables, arguments, or functions alike) coming close to typos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants