-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Suggest making protocol attribute read-only if invariance causes conflict #6002
Comments
We can also add a link to the docs (again, similarly to the invariant collection note). |
What are the chances that there will someday be a less verbose way of declaring a Protocol attribute read-only? moving from
to
is quite painful in terms of readability, especially when you've got 10 or so attributes that should all be considered read-only. |
I would say chances are low, but theoretically non-zero. |
Maybe we could support syntax like:
|
I would love that. I tried it but of course Protocols don't support it currently. I assume the hesitation there is around the fact that Final in its other contexts means that the value is assigned at class/module definition time, rather than at the time of object instantiation? |
More because being final and being read-only are different concepts: a read-only attribute can be overridden in a subclass, a final one can't. Final is a stronger concept. The notion of being final is in fact already quite complex (and has subtle differences in different languages), I don't want to overload it even more. |
This is suggested/recommended at python/mypy#6002 Discovered as part of matrix-org/synapse#15052
* Use a property to define `Jsonlibrary` This is suggested/recommended at python/mypy#6002 Discovered as part of matrix-org/synapse#15052 * Okay fine, coverage * Fix the JsonLibrary protocol This is fiddlier than I expected. * Test under mypy 1.0 * Include mypy in default `tox` jobs
* Update mypy and mypy-zope * Remove unused ignores These used to suppress ``` synapse/storage/engines/__init__.py:28: error: "__new__" must return a class instance (got "NoReturn") [misc] ``` and ``` synapse/http/matrixfederationclient.py:1270: error: "BaseException" has no attribute "reasons" [attr-defined] ``` (note that we check `hasattr(e, "reasons")` above) * Avoid empty body warnings, sometimes by marking methods as abstract E.g. ``` tests/handlers/test_register.py:58: error: Missing return statement [empty-body] tests/handlers/test_register.py:108: error: Missing return statement [empty-body] ``` * Suppress false positive about `JaegerConfig` Complaint was ``` synapse/logging/opentracing.py:450: error: Function "Type[Config]" could always be true in boolean context [truthy-function] ``` * Fix not calling `is_state()` Oops! ``` tests/rest/client/test_third_party_rules.py:428: error: Function "Callable[[], bool]" could always be true in boolean context [truthy-function] ``` * Suppress false positives from ParamSpecs ```` synapse/logging/opentracing.py:971: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]" [arg-type] synapse/logging/opentracing.py:1017: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]" [arg-type] ```` * Drive-by improvement to `wrapping_logic` annotation * Workaround false "unreachable" positives See Shoobx/mypy-zope#91 ``` tests/http/test_proxyagent.py:626: error: Statement is unreachable [unreachable] tests/http/test_proxyagent.py:762: error: Statement is unreachable [unreachable] tests/http/test_proxyagent.py:826: error: Statement is unreachable [unreachable] tests/http/test_proxyagent.py:838: error: Statement is unreachable [unreachable] tests/http/test_proxyagent.py:845: error: Statement is unreachable [unreachable] tests/http/federation/test_matrix_federation_agent.py:151: error: Statement is unreachable [unreachable] tests/http/federation/test_matrix_federation_agent.py:452: error: Statement is unreachable [unreachable] tests/logging/test_remote_handler.py:60: error: Statement is unreachable [unreachable] tests/logging/test_remote_handler.py:93: error: Statement is unreachable [unreachable] tests/logging/test_remote_handler.py:127: error: Statement is unreachable [unreachable] tests/logging/test_remote_handler.py:152: error: Statement is unreachable [unreachable] ``` * Changelog * Tweak DBAPI2 Protocol to be accepted by mypy 1.0 Some extra context in: - matrix-org/python-canonicaljson#57 - python/mypy#6002 - https://mypy.readthedocs.io/en/latest/common_issues.html#covariant-subtyping-of-mutable-protocol-members-is-rejected * Pull in updated canonicaljson lib so the protocol check just works * Improve comments in opentracing I tried to workaround the ignores but found it too much trouble. I think the corresponding issue is python/mypy#12909. The mypy repo has a PR claiming to fix this (python/mypy#14677) which might mean this gets resolved soon? * Better annotation for INTERACTIVE_AUTH_CHECKERS * Drive-by AUTH_TYPE annotation, to remove an ignore
Looks like this works in Pyright. It'll be nice to have this in Mypy too (or a different keyword to indicate read-only in Protocol). The |
If a concrete attribute has a narrower type than protocol attribute, mypy will complain (adapted from #5998):
It could help if mypy would suggest making the protocol attribute read-only (property or final attribute), which is often the right thing to do.
We already generate a related suggestion to use a covariant collection type when using an invariant collection causes type incompatibility.
The text was updated successfully, but these errors were encountered: