Skip to content

Typehint label / log / spinner / storage#458

Open
hippalectryon-0 wants to merge 4 commits intoQubesOS:mainfrom
hippalectryon-0:typehint-label
Open

Typehint label / log / spinner / storage#458
hippalectryon-0 wants to merge 4 commits intoQubesOS:mainfrom
hippalectryon-0:typehint-label

Conversation

@hippalectryon-0
Copy link

@hippalectryon-0 hippalectryon-0 commented Mar 16, 2026

Major: type-hint storage.py
Minor: type-hint label.py spinner.py log.py

Comments (for storage.py)

1. safe assert

In __lt__ we have

            if self._vid and other._vid:
                return (self._pool, self._vid) < (other._pool, other._vid)

Technically, _vid is supposed to be non-None if pool is non-None, not the other way around. Shouldn't it be if self.pool and other.pool: ?

Type error raise

In revert:

        if not isinstance(revision, str):
            raise TypeError('revision must be a str')

but typing def revert(self, revision: str) -> None: raises no type-checker error, indicating that it's never called with a non-str argument. Is the raise really necessary ?

None __str__

    def __str__(self):
        return self.name

but self.name can be None, and __str__ should never be None. I added an assert is not None for now.

LT assert

    def __lt__(self, other: object) -> bool:
        if isinstance(other, Pool):
            return self.name < other.name

What guarantees that self.name or other.name is defined here ?

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