Skip to content

DOC: Spell precommit #5475

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

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

Conversation

dave3d
Copy link
Member

@dave3d dave3d commented Jul 23, 2025

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

This PR address a spell checking pre-commit to ITK. So any time code is committed to an ITK repo, the SimpleITK/CommentSpellCheck (CSC) script is run on the new code. CSC checks the spelling of all comments in new file.

Previously CSC was run on ITK as a Github Action on the ITK repository. Moving to a pre-commit runs CSC on the local developer's system allowing for faster feedback and reducing Github server usage. Therefore, this PR removes .github/workflows/spell-check-comments.yml.

Also this PR includes many spelling fixes to the C++ files in the ITK code base. These bugs had not been address before because our main concern has been the spelling in the header files, which are used to create the documentation. With this new PR, the .cxx are also checked, so we had to fixed all the bugs already in ITK.

The .cxx fixes are split into multiple commits for organizational sake. All the commits in this PR can be squashed into one, if that is preferred.

UPDATE: I also meant to say that the version of CSC has been upgraded to the latest 0.4.3. This version uses a pure Python spelling backend, unlike the previous version. That previous version depended on the C enchant library, which was not available on all systems.

@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Documentation Documentation improvement or change area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:ThirdParty Issues affecting the ThirdParty module area:Video Issues affecting the Video module area:Numerics Issues affecting the Numerics module labels Jul 23, 2025
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Thank you Dave.

@github-actions github-actions bot added the area:Documentation Issues affecting the Documentation module label Jul 24, 2025
dave3d added 2 commits July 28, 2025 16:49
Now that the spell checker is being run on all the CXX files
in ITK, we need to fix some of the spell bugs that existed.
This PR address a spell checking pre-commit to ITK. So any time code is
committed to an ITK repo, the SimpleITK/CommentSpellCheck (CSC) script is
run on the new code. CSC checks the spelling of all comments in new file.

Also the version of CSC use is upgraded to 0.4.3, which uses a pure Python
spelling backend, unlike the previous version. That previous version depended
on the C enchant library, which was not available on all systems.

We have removed the spell checking github action, since it is no longer
required.  Any time a pull request is submitted to github, pre-commit
is run.
@dave3d
Copy link
Member Author

dave3d commented Jul 29, 2025

I think it's ready to go.

@dave3d dave3d requested review from dzenanz and blowekamp July 29, 2025 15:21
Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

There are many legitimate additions to the dictionary, but some which should not be there. I pointed out the first 3 I found.

Cz
DDataFrameobject
Daniesson
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a misspelling.

@@ -64,45 +76,61 @@ Elmaghraby
Englewood
Eun
Evol
ExhaustiveOptv
Copy link
Member

Choose a reason for hiding this comment

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

Why not rename this, like many other things? It appears in a test, and is therefore not part of the public API.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I exclude all tests from the spell checking?

Copy link
Member

Choose a reason for hiding this comment

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

That would only make sense as a temporary measure. This PR is already big. You could disable spell check in tests, and add an issue to enable it. Then tackle it when you (or someone else) has enthusiasm and time to spare.

Hideaki
Hierarchique
Hilden
Hiraki
Hirschberg
Hmmm
Copy link
Member

Choose a reason for hiding this comment

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

This is not a necessary part of a comment here, and could be removed instead of added to the dictionary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:ThirdParty Issues affecting the ThirdParty module area:Video Issues affecting the Video module type:Documentation Documentation improvement or change type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants