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

assertIn false positivies #3430

Closed
nijel opened this issue May 24, 2024 · 9 comments · May be fixed by #3470
Closed

assertIn false positivies #3430

nijel opened this issue May 24, 2024 · 9 comments · May be fixed by #3470

Comments

@nijel
Copy link

nijel commented May 24, 2024

Codespell 2.3.0 started to complain about assertIn method:

weblate/lang/tests.py:330: assertIn ==> asserting, assert in, assertion
weblate/lang/tests.py:331: assertIn ==> asserting, assert in, assertion

I know I can allow this particular word, but complaining on standard Python test suite API seems a bit annoying.

@DimitriPapadopoulos
Copy link
Collaborator

How about moving it to the code dictionary? Merge requests welcome.

@CodyCBakerPhD
Copy link

@DimitriPapadopoulos

How about moving it to the code dictionary? Merge requests welcome.

Happy to try to fix this, tracked it down to this entry in the global dict: https://github.com/codespell-project/codespell/blob/master/codespell_lib/data/dictionary.txt#L5633

What I'm not clear on is how moving this to dictionary_code would help; it seems what we would want it to do is ignore the specific capitalization of .assertIn (either pre-ambled by self or TestCase) when in .py files

Is there a separate place in codespell to specify conditional code-related ignores of items from the dictionary that would otherwise make sense in normal text?

@DimitriPapadopoulos
Copy link
Collaborator

By design, codespell is mostly case-insensitive and doesn't make a difference between assertIn and assertin.

@CodyCBakerPhD
Copy link

OK, so how would I define an ignore rule for assertin when scanning Python code?

@DimitriPapadopoulos
Copy link
Collaborator

There's the code dictionary. Because codespell is language agnostic, you cannot define a rule for Python files only.

@CodyCBakerPhD
Copy link

The code dictionary seems to have definitions of common misspellings of code, e.g.,

agrv->argv
...
arange->arrange, a range,

these make sense

The problem is that a simple pure Python built-in snippet like

import unittest

class MyTest(unittest.TestCase):
    def some_test(self):
        self.assertIn("a", ["b"])

makes codespell think that the code line is a misspelling with suggestions corresponding to the non-code dictionary: https://github.com/codespell-project/codespell/blob/master/codespell_lib/data/dictionary.txt#L5633

So are you saying that this is a broader issue that the non-code dictionary is running and making suggestions on code instead of just normal text?

@DimitriPapadopoulos
Copy link
Collaborator

DimitriPapadopoulos commented May 24, 2024

Everything's text as far as codespell is concerned, whether Shakespeare prose or source code.

You just get to select the typo lists/dictionaries you want to use - the default ones or your own selection. The code dictionary contains typos that are invalid English words in a general context, but are valid words in the context of source code. You're supposed to avoid the code dictionary when spellchecking source code - or use it at the expense of more false positives in the hope of catching a few true positives.

jneo8 added a commit to jneo8/charmed-openstack-upgrader that referenced this issue May 27, 2024
jneo8 added a commit to jneo8/charmed-openstack-upgrader that referenced this issue May 27, 2024
jneo8 added a commit to canonical/charmed-openstack-upgrader that referenced this issue May 28, 2024
jneo8 added a commit to canonical/charmed-openstack-upgrader that referenced this issue May 28, 2024
ca-scribner added a commit to canonical/grafana-agent-k8s-operator that referenced this issue May 28, 2024
ca-scribner added a commit to canonical/grafana-agent-k8s-operator that referenced this issue May 28, 2024
ca-scribner added a commit to canonical/grafana-agent-k8s-operator that referenced this issue May 28, 2024
gadomski added a commit to stac-utils/pystac that referenced this issue May 29, 2024
github-merge-queue bot pushed a commit to stac-utils/pystac that referenced this issue May 29, 2024
* build(deps): bump ruff from 0.4.5 to 0.4.6

Bumps [ruff](https://github.com/astral-sh/ruff) from 0.4.5 to 0.4.6.
- [Release notes](https://github.com/astral-sh/ruff/releases)
- [Changelog](https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md)
- [Commits](astral-sh/ruff@v0.4.5...v0.4.6)

---
updated-dependencies:
- dependency-name: ruff
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* deps: keep codespell below 2.3

Because of codespell-project/codespell#3430

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Pete Gadomski <[email protected]>
gforcada added a commit to plone/meta that referenced this issue Jun 5, 2024
And ensure `assertIn` is ignored, as that gives LOTS of false positives.

Hopefully the upstream issue is solved:

codespell-project/codespell#3430
@peternewman
Copy link
Collaborator

How about moving it to the code dictionary? Merge requests welcome.

I've opened #3469 for this specific aspect.

The code dictionary seems to have definitions of common misspellings of code, e.g.,

agrv->argv

This shouldn't be in there as it will likely rarely be triggered, if you're checking code you won't use this dictionary (but it might not pass CI in the normal one where it would work because argv isn't a word in a proper dictionary.

arange->arrange, a range,

This is a numpy function, so wants skipping if you're checking code (so it doesn't become arrange or a range):
https://numpy.org/doc/stable/reference/generated/numpy.arange.html

@DimitriPapadopoulos
Copy link
Collaborator

Can be closed after moving assertin to code.

For agrv->argv and similar entries, moved from code to main dictionary in #3470.

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 a pull request may close this issue.

4 participants