Skip to content

Replace direct type checks with isinstance - #2717

Open
BharatDeva wants to merge 1 commit into
certtools:developfrom
BharatDeva:fix/use-isinstance-2636
Open

Replace direct type checks with isinstance#2717
BharatDeva wants to merge 1 commit into
certtools:developfrom
BharatDeva:fix/use-isinstance-2636

Conversation

@BharatDeva

Copy link
Copy Markdown

Description

Fixes #2636.

This replaces direct type(...) is ... checks with isinstance(...) in the code paths listed in the issue. The affected checks are simple builtin or IntelMQ message type checks, so isinstance keeps the intended behavior while following the preferred Python style.

One exact class comparison remains in Message.__eq__:

type_eq = type(self) is type(other)

That comparison is intentional because message equality should still distinguish different Message subclasses after confirming that other is a Message instance.

Testing

Ran on WSL Ubuntu with Python 3.12.3:

  • PYTHONPATH=. .venv-wsl-pr/bin/python -m pytest -o addopts= intelmq/tests/lib/test_message.py intelmq/tests/lib/test_pipeline.py intelmq/tests/lib/test_utils.py
    • 155 passed, 7 skipped
  • PYTHONPATH=. .venv-wsl-pr/bin/python -m pytest -o addopts= intelmq/tests/bots/experts/format_field/test_expert.py intelmq/tests/bots/parsers/generic/test_parser_csv.py intelmq/tests/bots/parsers/html_table/test_parser.py
    • 12 passed, 3 skipped
  • PYTHONPATH=. .venv-wsl-pr/bin/python -m compileall -q intelmq
    • passed
  • git diff --check
    • passed

Use isinstance() for direct type comparisons across the code paths listed in issue certtools#2636. This keeps the behavior the same for the checked builtin and IntelMQ message types while following the preferred Python style and allowing subclasses where that is appropriate.

The exact class equality check in Message.__eq__ is intentionally left as type(self) is type(other), because it distinguishes different Message subclasses after confirming that the other object is a Message instance.

Signed-off-by: BharatDeva <278575558+BharatDeva@users.noreply.github.com>
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.

Replace type() by isinstance()

1 participant