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

mypy: fix type checking error #3365

Merged

Conversation

davidculley
Copy link
Contributor

To have an easier time in my other pull requests, when figuring out which type errors were introduced by me and which already existed, I'm fixing some of the type errors on the current main branch.

This pull request fixes this mypy error:

error: Argument 1 to "_answerCard" of "Reviewer" has incompatible type "int"; expected "Literal[1, 2, 3, 4]" [arg-type]

The function _answerCard wants only numbers 1, 2, 3 or 4 (that is, a variable of type Literal[1, 2, 3, 4]. But we're passing a variable of type int which can hold many more numbers than these mentioned four.

I'm basically just

  • casting int to Literal[1, 2, 3, 4] (which by itself would already suffice)
  • adding a check that the given number is indeed one of the four numbers, so that we don't mute mypy only to miss runtime errors when there actually is one because mypy stayed silent since we muted it.

error: Argument 1 to "_answerCard" of "Reviewer" has incompatible type "int"; expected "Literal[1, 2, 3, 4]"  [arg-type]
def generate_default_answer_keys():
for ease in aqt.mw.pm.default_answer_keys:
key = aqt.mw.pm.get_answer_key(ease)
if key:
Copy link
Member

Choose a reason for hiding this comment

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

Walrus operator would make this more concise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should key only be not None or does it also need to be different from the empty string?

In other words: I would like to change if key: to if key is not None:, provided this doesn't change its meaning.

Copy link
Member

Choose a reason for hiding this comment

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

The user can enter an empty string in the preferences, so we also need to guard against that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for fixing the code. I wasn't sure about that.

qt/aqt/reviewer.py Outdated Show resolved Hide resolved
qt/aqt/reviewer.py Outdated Show resolved Hide resolved
@dae
Copy link
Member

dae commented Aug 29, 2024

🚀

@dae dae merged commit b35b69a into ankitects:main Aug 29, 2024
1 check passed
@davidculley davidculley deleted the feature/fix-type-checking-int-to-literal branch August 29, 2024 22:54
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.

2 participants