Skip to content

chore: fix typing of cloudinit.templater - #6989

Open
Mohith1612 wants to merge 1 commit into
canonical:mainfrom
Mohith1612:typing-templater
Open

chore: fix typing of cloudinit.templater#6989
Mohith1612 wants to merge 1 commit into
canonical:mainfrom
Mohith1612:typing-templater

Conversation

@Mohith1612

Copy link
Copy Markdown

Proposed Commit Message

chore: fix typing of cloudinit.templater

JinjaSyntaxParsingException.format_error_message declared line_number
as str, but the only production caller, __str__, passes self.lineno,
which is an int. The annotation was wrong.

Two test call sites had been written to match the wrong annotation and
pass the string "2". Correct them to pass an int. Output is unchanged,
since message_template renders line_number through str.format, so "2"
and 2 produce the same string.

With that fixed, cloudinit.templater and
tests.unittests.test_templating both pass check_untyped_defs, so drop
them from the mypy override list in pyproject.toml.

Refs GH-5445

Additional Context

Refs GH-5445.

There are six call sites of format_error_message across five modules.
Four pass an int, including the only production one, and two pass a string:

Call site Passes
cloudinit/templater.py:49 (__str__) self.lineno, an int
tests/unittests/test_templating.py:270 4
tests/unittests/test_templating.py:289 4
tests/unittests/config/test_schema.py:1014 3
tests/unittests/cmd/test_query.py:618 "2"
tests/unittests/cmd/devel/test_render.py:157 "2"

The two string call sites were the ones written against the incorrect
annotation. They sit in modules mypy already checks, which is why correcting
the annotation surfaces them. I went with int rather than widening to
Union[int, str], since a line number is an int and the production caller
already treats it as one.

The mismatch went unnoticed because message_template renders {line_number}
through str.format, so both types produce identical output. No assertion text
changes.

I kept this to the annotation and the call sites. The other functions in
templater.py are untouched.

One possible follow-up, not included here: line_content is declared
str = "", but tests/unittests/test_templating.py:282 parametrizes it over
("", None), so Optional[str] would be more accurate. It does not affect any
mypy result, so I left it out. Happy to do that as a separate PR if it's wanted.

Test Steps

Typing change with no runtime behaviour change, so no new tests. The existing
tests at tests/unittests/test_templating.py:263-291 already exercise
format_error_message with an int line number, which is what showed the
annotation was wrong.

Local runs on this branch:

$ tox -e py3
5742 passed, 5 skipped, 13 xfailed, 10 warnings in 154.44s

$ tox -e check_format
ruff: All checks passed!
pylint: Your code has been rated at 10.00/10
black: 595 files would be left unchanged.
mypy: Success: no issues found in 591 source files
congratulations :)

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

JinjaSyntaxParsingException.format_error_message declared line_number
as str, but the only production caller, __str__, passes self.lineno,
which is an int. The annotation was wrong.

Two test call sites had been written to match the wrong annotation and
pass the string "2". Correct them to pass an int. Output is unchanged,
since message_template renders line_number through str.format, so "2"
and 2 produce the same string.

With that fixed, cloudinit.templater and
tests.unittests.test_templating both pass check_untyped_defs, so drop
them from the mypy override list in pyproject.toml.

Refs canonicalGH-5445
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.

1 participant