Repository navigation
Build variable replacement dict on init - #4731
Conversation
| # pylint: disable=unused-import | ||
| from qutebrowser.mainwindow import tabbedbrowser | ||
| # pylint: enable=unused-import | ||
| ReplacementFunction = typing.Callable[['tabbedbrowser.TabbedBrowser'], str] |
There was a problem hiding this comment.
| ReplacementFunction = typing.Callable[['tabbedbrowser.TabbedBrowser'], str] | |
| _ReplacementFunction = typing.Callable[['tabbedbrowser.TabbedBrowser'], str] |
as it shouldn't be needed from outside of this module
| 'url': lambda: url().toString( | ||
| def _init_variable_replacements() -> typing.Dict[str, ReplacementFunction]: | ||
| """Return a dict from variable replacements to fns processing them.""" | ||
| url = _current_url |
There was a problem hiding this comment.
Could just rename the _current_url function to _url instead 😉
| return replacements | ||
|
|
||
|
|
||
| VARIABLE_REPLACEMENTS = types.MappingProxyType(_init_variable_replacements()) |
There was a problem hiding this comment.
Likely to ensure that it is not accidentally changed in some other places.
However I think it would be better to mark its type as Mapping[...] for a "compile-time" check.
There was a problem hiding this comment.
Mapping is just a more loose version of Dict (like Sequence or Iterable compared to List), I don't think there's currently a way to make mypy check immutability.
As for using types.MappingProxyType: I don't think this makes sense here. The name clearly conveys that it's constant, so if it'd be modified somewhere else, that'd stand out anyways. That kind of stuff IMHO makes sense where accidental mutations can happen easily (like with objects coming from the config), but I don't think that's the case here.
There was a problem hiding this comment.
No, MutableMapping is a more loose version of Dict. Mapping is immutable.
Also, that works in mypy.
There was a problem hiding this comment.
There's also from typing_extensions import Final (see python/mypy#5522 oops, accidentally referenced that PR for usage), but that is another kind of immutability - that the variable cannot be reassigned. It's not the case here.
720c8f3 to
511bed4
Compare
|
Unfortunately, mypy doesn't type check dynamically typed functions. So using |
My bad, you're right!
Since we're annotating more and more code in qutebrowser, I'd still go ahead with using |
|
Thanks! |
Essentially a little more fancy version of http://www.github.com/qutebrowser/qutebrowser/pull/4536
Continuation of #4652 (comment)
Haven't run all tests on this yet, but it seemed to pass what I thought would break. I'll fix anything if it comes up though (still not 100% sure this is safe).
This change is