Skip to content

Add typos action#645

Open
PodushkaPIR wants to merge 8 commits intoDesbordante:mainfrom
PodushkaPIR:add-typeos
Open

Add typos action#645
PodushkaPIR wants to merge 8 commits intoDesbordante:mainfrom
PodushkaPIR:add-typeos

Conversation

@PodushkaPIR
Copy link

@PodushkaPIR PodushkaPIR commented Dec 10, 2025

  • Add base typos-check in CI and put tool.typos in pyproject.toml
  • Fix suggestion from typos

As it turns out, typos does not yet support layered configurations for working with specific directories and files, although there issues about this. Therefore, I had to create one general configuration file for the entire project. In this regard, some rules in the pyproject.toml file may seem strange.

If it is better to add some files to the ignore list or get it from, I am open for suggestions.

Copy link
Collaborator

@BUYT-1 BUYT-1 left a comment

Choose a reason for hiding this comment

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

Looks good, though I'd prefer a more granular approach. Ideally, even more granular than file-by-file basis, whitelisting each false positive explicitly. I'm thinking extend-ignore-re with enough context that it will never reasonably appear in another text or that it will always be a typo (e.g. \"mispelling\" is a misspelling). We can always add stuff to whitelists.

pyproject.toml Outdated
Comment on lines +64 to +67
abd = "abd"
ba = "ba"
countr = "countr"
helo = "helo"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are these ignored in all project files?

Copy link
Author

Choose a reason for hiding this comment

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

Because there is only one config and the rules from the config should apply to all files in a good way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://github.com/crate-ci/typos/blob/master/docs/reference.md:

Configuration is read from the following (in precedence order)

  • Command line arguments
  • File specified via --config PATH
  • Search parents of specified file / directory for one of typos.toml, _typos.toml, .typos.toml, Cargo.toml, or pyproject.toml.
    • In pyproject.toml, the below fields must be under the [tool.typos] section. If this section does not
      exist, the config file will be skipped.
    • In Cargo.toml, the below fields must be under either [workspace.metadata.typos] or [package.metadata.typos]

Does this action apply only the first one it detects from the list or all of them but in order?

Copy link
Author

Choose a reason for hiding this comment

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

I tested and action applied only one config which was detected (if I understood the question correctly)

pyproject.toml Outdated
countr = "countr"
helo = "helo"
nd = "nd"
identificator = "identificator"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this word excluded? "identificator" is not a real word, "identifier" and "identification" are.

Copy link
Author

Choose a reason for hiding this comment

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

Yes! I will fix this.

"test_input_data"
]

[tool.typos.default.extend-words]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to only ignore words per-file/directory, not project-wide. Let's say, ignore "ND" for src/core/algorithms/nd/*, ignore "nd" for the Python ND example, and other ND-related places, and don't ignore it in others (extend-ignore-words-re seems to not ignore case, so that can be used).

Copy link
Author

@PodushkaPIR PodushkaPIR Dec 24, 2025

Choose a reason for hiding this comment

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

Okay, I'll do that. However, since typos can't apply specific rules to specific files and directories, I think it needs to use a matrix in CI with different scenarios.

Copy link
Author

Choose a reason for hiding this comment

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

Would it be a good idea to create different files for specific scenarios (e.g. typos_nd.toml, typos_other.toml, etc)?

pyproject.toml Outdated
Comment on lines +56 to +57
"README.md",
"/README_PYPI.md",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are typos ignored in README files?

pyproject.toml Outdated
extend-exclude = [
"README.md",
"/README_PYPI.md",
"examples/notebooks",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a tough one. Would it be too hard to dump notebook cell text to files (.py and .md so the action understands) and check those instead?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you're right.

pyproject.toml Outdated
Comment on lines +59 to +60
"examples/datasets",
"test_input_data"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should have a more granular whitelist instead.

ba = "ba"
countr = "countr"
helo = "helo"
nd = "nd"
Copy link
Collaborator

Choose a reason for hiding this comment

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

"nd" instead of "and" is a legitimate typo, though

@BUYT-1 BUYT-1 changed the title Add typos Add typos action Dec 14, 2025
@BUYT-1
Copy link
Collaborator

BUYT-1 commented Jan 22, 2026

If it's too much of a pain to do fully, maybe we can merge as is and deal with the stuff I've pointed out later.

@PodushkaPIR
Copy link
Author

If it's too much of a pain to do fully, maybe we can merge as is and deal with the stuff I've pointed out later.

No, it shouldn't be difficult, I'm a little busy right now, sorry(
I'll try to merge all your points

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.

2 participants