Skip to content

🏷️ Make Parameter.name typed as str instead of str | None#1878

Draft
YuriiMotov wants to merge 1 commit into
masterfrom
Parameter-typing-improvements
Draft

🏷️ Make Parameter.name typed as str instead of str | None#1878
YuriiMotov wants to merge 1 commit into
masterfrom
Parameter-typing-improvements

Conversation

@YuriiMotov

Copy link
Copy Markdown
Member

Description

On attempt to upgrade ty to 0.0.48+ it gives:

warning[unused-type-ignore-comment]: Unused blanket `type: ignore` directive
   --> typer/_click/core.py:924:38
    |
924 |         value = opts.get(self.name)  # type: ignore
    |                                      ^^^^^^^^^^^^^^
    |
help: Remove the unused suppression comment
921 |     def consume_value(
922 |         self, ctx: Context, opts: Mapping[str, Any]
923 |     ) -> tuple[Any, ParameterSource]:
    -         value = opts.get(self.name)  # type: ignore
924 +         value = opts.get(self.name)
925 |         source = ParameterSource.COMMANDLINE
926 |
927 |         if value is None:

This is obviously false-negative. self.name is typed as str | None, so it can't be passed to opts.get() that accepts str.

While investigatig this I found out that Click recently updated the typing of Parameter.name to just str. See PR pallets/click#2805

So, why don't we do the same?

AI Disclaimer

Implemented with the help of Claude Opus 4.8 (1M context), then reviewed manually

Checklist

  • This PR links to a GitHub Discussion for the proposed code change.
  • I added tests for the change.
  • The new or updated tests fail on the main branch and pass on this PR.
  • Coverage stays at 100%.
  • The documentation explains the change if needed.

(typing only, no tests needed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants