Skip to content

Fix some typos - #5118

Merged
merelcht merged 12 commits into
kedro-org:mainfrom
yarikoptic:enh-codespell
Nov 25, 2025
Merged

merelcht merged 12 commits into
kedro-org:mainfrom
yarikoptic:enh-codespell

Conversation

@yarikoptic

Copy link
Copy Markdown
Contributor

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.

Note that it fixes pattens in a signature (and body) of an internal function

@merelcht

Copy link
Copy Markdown
Member

Hi @yarikoptic, thanks for this suggestion. We have "Vale" in place too to check spelling for markdown files. Before adding in another tool, I think we should check if there's any overlap and if not make sure they check things in similar ways to keep spelling consistency across python and markdown files.

Comment on lines -292 to -294
config = {"filepath": "{nam}.csv"}
config = {"filepath": "{name}.csv"}
CatalogConfigResolver._validate_pattern_config(pattern, config)
# DatasetError: Incorrect dataset configuration provided. Keys used in the configuration {'{nam}'} should present in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to be an intentional misspelling

@yarikoptic yarikoptic Sep 24, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

gotcha - fixed

Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
=== Do not change lines below ===
{
 "chain": [],
 "cmd": "sed -i -e 's,\\(fo=conf_source.*\\),\\1  # codespell:ignore,g' ./kedro/config/omegaconf_config.py",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
@yarikoptic

Copy link
Copy Markdown
Contributor Author

@merelcht note that codespell does not actually "spell" -- it just fixes typos it knows about. So there should be less false positives. But also it checks all and any text file(s) it finds, so not language/format aware/specific at all. Altogether could be considered complimentary. Your call

…nteractively

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "codespell -w -i 3 -C 4 ./tests/framework/project/test_logging.py ./kedro/io/catalog_config_resolver.py",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^

Signed-off-by: Yaroslav Halchenko <debian@onerussian.com>
…os automagically

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

Copy link
Copy Markdown
Member

Hi @yarikoptic, thanks for this suggestion. We have "Vale" in place too to check spelling for markdown files. Before adding in another tool, I think we should check if there's any overlap and if not make sure they check things in similar ways to keep spelling consistency across python and markdown files.

Overall, Vale is more of a prose style linter, while Codespell is a minimalist spell checker. That said, Vale does have a spell checker, too. But why is it off for Markdown? 🤔

Vale.Spelling = NO

The other problem with Vale IMO is that it gives so many suggestions that people just ignore them Looking at the logs on https://github.com/kedro-org/kedro/actions/runs/16328163642/job/46123623172, there are even some ERROR severity things flagged (not introduced by the PR).

I think it's fine to use Vale for spelling if it works, but it's worth understanding why it's not flagging some of these obvious issues.

@ankatiyar

Copy link
Copy Markdown
Contributor

@deepyaman The spell check is on, it just uses the Kedro config https://github.com/kedro-org/kedro/blob/main/.github/styles/Kedro/Spellings.yml instead of the default Vale one.

@ankatiyar

Copy link
Copy Markdown
Contributor

We already have Vale as a spell checker for docs, but since we added it, it only ran on the new additions which is why I think some of these typos were left out. I suppose codespell also checks for typos in the code? I'd be in favour of accepting the corrections for the typos but leaving out codespell for now. It seems promising but we already have a tool for spell checking, it might be overkill?

@merelcht

Copy link
Copy Markdown
Member

I agree with @ankatiyar ! Let's include the fixes, but leave codespell out for now. If that's okay with you @yarikoptic, we can clean up the PR and get it merged.

merelcht and others added 3 commits November 25, 2025 12:12
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
@merelcht merelcht changed the title Add codespell support (config, workflow to detect/not fix) and make it fix some typos Fix some typos Nov 25, 2025
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>
Signed-off-by: Merel Theisen <merel.theisen@quantumblack.com>

@ankatiyar ankatiyar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @merelcht and @yarikoptic

@merelcht
merelcht enabled auto-merge (squash) November 25, 2025 13:05

@ElenaKhaustova ElenaKhaustova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks 👍

Signed-off-by: ElenaKhaustova <157851531+ElenaKhaustova@users.noreply.github.com>
@merelcht
merelcht merged commit 611760a into kedro-org:main Nov 25, 2025
34 of 35 checks passed
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.

5 participants