Skip to content

Commit

Permalink
Adjust some type hints to better match parent classes
Browse files Browse the repository at this point in the history
mypy 1.11 has stricter checking of the type signature overridden methods: https://github.com/python/mypy/blob/master/CHANGELOG.md#stricter-checks-for-untyped-overrides

There's a couple of places where I added type hints and had to duplicate the
default kwarg value from the parent.

In `completionmodel.py` it was complaining that the type signature of
`parent()` didn't match that of `QAbstractItemModel` and `QObject`. I've
changed it to be happy, and incidently made it so the positional arg is
optional, otherwise it's impossible to call `QObject.parent()`. Options that I
see:

1. support both variant of parent() - what I've done, the technically correct
   solution
2. have the two overload definitions but in the actual implementation make the
   positional argument required - would mean one overload signature was a lie,
   but would make it more clear how to call `CompletionModel.parent()
3. do type: ignore[override] and leave it as it was

In the end I don't expect there to be many callers of
`CompletionModel.parent(child)`.

I also added a few more function type hints to `completionmodel.py` while I
was there. Not all of them though!

In `objreg.py` I expanded the user of `_IndexType` because as
7b9d702 say, the window register uses int as the key.
  • Loading branch information
toofar committed Aug 12, 2024
1 parent 59b2cc7 commit e2f718a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 14 deletions.
2 changes: 1 addition & 1 deletion qutebrowser/browser/downloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,7 @@ def headerData(self, section, orientation, role=Qt.ItemDataRole.DisplayRole):
else:
return ""

def data(self, index, role):
def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole) -> Any:
"""Download data from DownloadManager."""
if not index.isValid():
return None
Expand Down
6 changes: 3 additions & 3 deletions qutebrowser/browser/network/proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

"""Handling of proxies."""

from typing import Optional
from typing import Optional, List

from qutebrowser.qt.core import QUrl, pyqtSlot
from qutebrowser.qt.network import QNetworkProxy, QNetworkProxyFactory
from qutebrowser.qt.network import QNetworkProxy, QNetworkProxyFactory, QNetworkProxyQuery

from qutebrowser.config import config, configtypes
from qutebrowser.utils import message, usertypes, urlutils, utils, qtutils
Expand Down Expand Up @@ -71,7 +71,7 @@ def _set_capabilities(self, proxy):
capabilities &= ~lookup_cap
proxy.setCapabilities(capabilities)

def queryProxy(self, query):
def queryProxy(self, query: QNetworkProxyQuery = QNetworkProxyQuery()) -> List[QNetworkProxy]:
"""Get the QNetworkProxies for a query.
Args:
Expand Down
25 changes: 18 additions & 7 deletions qutebrowser/completion/models/completionmodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

"""A model that proxies access to one or more completion categories."""

from typing import MutableSequence
from typing import MutableSequence, overload, Optional, Any

from qutebrowser.qt.core import Qt, QModelIndex, QAbstractItemModel
from qutebrowser.qt.core import Qt, QModelIndex, QAbstractItemModel, QObject

from qutebrowser.utils import log, qtutils, utils
from qutebrowser.api import cmdutils
Expand All @@ -30,7 +30,7 @@ def __init__(self, *, column_widths=(30, 70, 0), parent=None):
self.column_widths = column_widths
self._categories: MutableSequence[QAbstractItemModel] = []

def _cat_from_idx(self, index):
def _cat_from_idx(self, index: QModelIndex):
"""Return the category pointed to by the given index.
Args:
Expand All @@ -48,7 +48,7 @@ def add_category(self, cat):
"""Add a completion category to the model."""
self._categories.append(cat)

def data(self, index, role=Qt.ItemDataRole.DisplayRole):
def data(self, index: QModelIndex, role: int = Qt.ItemDataRole.DisplayRole) -> Any:
"""Return the item data for index.
Override QAbstractItemModel::data.
Expand All @@ -74,7 +74,7 @@ def data(self, index, role=Qt.ItemDataRole.DisplayRole):
idx = cat.index(index.row(), index.column())
return cat.data(idx)

def flags(self, index):
def flags(self, index: QModelIndex) -> Qt.ItemFlag:
"""Return the item flags for index.
Override QAbstractItemModel::flags.
Expand All @@ -91,7 +91,7 @@ def flags(self, index):
# category
return Qt.ItemFlag.NoItemFlags

def index(self, row, col, parent=QModelIndex()):
def index(self, row: int, col: int, parent: QModelIndex = QModelIndex()) -> QModelIndex:
"""Get an index into the model.
Override QAbstractItemModel::index.
Expand All @@ -108,14 +108,25 @@ def index(self, row, col, parent=QModelIndex()):
return self.createIndex(row, col, self._categories[parent.row()])
return self.createIndex(row, col, None)

def parent(self, index):
@overload
def parent(self, index: QModelIndex) -> QModelIndex:
...

@overload
def parent(self) -> Optional[QObject]:
...

def parent(self, index=None):
"""Get an index to the parent of the given index.
Override QAbstractItemModel::parent.
Args:
index: The QModelIndex to get the parent index for.
"""
if not index:
return QObject.parent(self)

parent_cat = index.internalPointer()
if not parent_cat:
# categories have no parent
Expand Down
6 changes: 3 additions & 3 deletions qutebrowser/utils/objreg.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def __setitem__(self, name: _IndexType, obj: Any) -> None:

super().__setitem__(name, obj)

def __delitem__(self, name: str) -> None:
def __delitem__(self, name: _IndexType) -> None:
"""Extend __delitem__ to disconnect the destroyed signal."""
self._disconnect_destroyed(name)
super().__delitem__(name)
Expand All @@ -101,7 +101,7 @@ def _disconnect_destroyed(self, name: _IndexType) -> None:
pass
del partial_objs[name]

def on_destroyed(self, name: str) -> None:
def on_destroyed(self, name: _IndexType) -> None:
"""Schedule removing of a destroyed QObject.
We don't remove the destroyed object immediately because it might still
Expand All @@ -111,7 +111,7 @@ def on_destroyed(self, name: str) -> None:
log.destroy.debug("schedule removal: {}".format(name))
QTimer.singleShot(0, functools.partial(self._on_destroyed, name))

def _on_destroyed(self, name: str) -> None:
def _on_destroyed(self, name: _IndexType) -> None:
"""Remove a destroyed QObject."""
log.destroy.debug("removed: {}".format(name))
if not hasattr(self, 'data'):
Expand Down

0 comments on commit e2f718a

Please sign in to comment.