Skip to content

Use staticmethod for PasswordStrength methods#4461

Merged
svartkanin merged 1 commit intoarchlinux:masterfrom
correctmost:cm/use-staticmethod
Apr 22, 2026
Merged

Use staticmethod for PasswordStrength methods#4461
svartkanin merged 1 commit intoarchlinux:masterfrom
correctmost:cm/use-staticmethod

Conversation

@correctmost
Copy link
Copy Markdown
Contributor

PR Description:

The methods don't need access to the class, so they don't need to be classmethods. This change reduces the number of 'bad return' warnings seen when running Pyright, Pyrefly, and ty.

Example warnings fixed:

# Pyright
archinstall/lib/models/users.py:61:13 - error: Type "Literal[PasswordStrength.STRONG]" is not assignable to return type "Self@PasswordStrength"
    Type "Literal[PasswordStrength.STRONG]" is not assignable to type "Self@PasswordStrength" (reportReturnType)

# Pyrefly
archinstall/lib/models/users.py:61:13-23: Returned type `Literal[PasswordStrength.STRONG]` is not assignable to declared return type `Self@PasswordStrength` [bad-return]

# ty
archinstall/lib/models/users.py:61:13: error[invalid-return-type] Return type does not match returned value: expected `Self@_check_password_strength`, found `Literal[PasswordStrength.STRONG]`

Tests and Checks

  • I have tested the code!

The methods don't need access to the class, so they don't need to
be classmethods. This change reduces the number of 'bad return'
warnings seen when running Pyright, Pyrefly, and ty.
@correctmost correctmost requested a review from Torxed as a code owner April 22, 2026 06:41
@svartkanin svartkanin merged commit d176532 into archlinux:master Apr 22, 2026
9 checks passed
@correctmost correctmost deleted the cm/use-staticmethod branch April 22, 2026 11:33
@codefiles
Copy link
Copy Markdown
Contributor

@correctmost, I submitted now merged pull requests to use typing.Self where applicable. This pull request fixed a location where I applied it incorrectly. There are a few more places I applied it incorrectly that are also detected by Pyright, Pyrefly, and ty. I will make sure to run Pyright, Pyrefly, and ty before submitting pull requests. Are you using any particular configuration for these with this repo?

@correctmost
Copy link
Copy Markdown
Contributor Author

I submitted now merged pull requests to use typing.Self where applicable. This pull request fixed a location where I applied it incorrectly.

There's disagreement across type checkers about whether typing.Self is permissible here: astral-sh/ty#2915. I decided to make the @staticmethod change because I think it makes sense from a code-clarity perspective and because it has the added benefit of reducing noise with Pyright, Pyrefly, and ty. I also think typing.Self can be tricky to reason about (the discussion on zubanls/zuban#304 about typing.Self in archinstall's Size class illustrates that.)

I will make sure to run Pyright, Pyrefly, and ty before submitting pull requests.

You will definitely encounter some false positives :). Pyright is the most mature, but it's seeing less development these days. Pyrefly and ty are under active development and still have gaps in their implementations of the typing spec: https://github.com/python/typing/blob/main/conformance/results/results.html. I have also spent some time getting archinstall false positives fixed in zuban: https://github.com/zubanls/zuban/issues?q=is%3Aissue%20state%3Aclosed%20archinstall

Are you using any particular configuration for these with this repo?

I am using the default settings for ty check and pyrefly check. For Pyright, I am using a config similar to the one from PR #3647.

I would ultimately like to make it easier to use non-mypy type checkers with archinstall because they have decent IDE integrations. I planned on chipping away at the warnings, like I did with this PR, and getting some configurations committed to pyproject.toml. Feel free to make similar changes!

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.

3 participants