Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use a property to define JsonLibrary #57

Merged
merged 5 commits into from
Feb 15, 2023
Merged

Use a property to define JsonLibrary #57

merged 5 commits into from
Feb 15, 2023

Conversation

DMRobertson
Copy link
Contributor

This is suggested/recommended at
python/mypy#6002

Discovered as part of matrix-org/synapse#15052

Use a property to define Jsonlibrary
20a7e94 
This is suggested/recommended at
python/mypy#6002

Discovered as part of matrix-org/synapse#15052
@DMRobertson DMRobertson requested a review from a team as a code owner February 15, 2023 17:53
@DMRobertson
Copy link
Contributor Author

I have snuck in two changes:

@DMRobertson DMRobertson requested a review from clokep February 15, 2023 19:44
clokep
clokep reviewed Feb 15, 2023
View reviewed changes
src/canonicaljson/__init__.py
JSONEncoder: Encoder
class JsonLibrary(Protocol): # pragma: no cover
@property
def JSONEncoder(self) -> Type[Encoder]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would JSONEncoder: Type[Encoder] have fixed it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! Not AFAICS.

dmr on titan in python-canonicaljson on  dmr/typing-jsonmodule via 🐍 v3.11.1 (python-3.11.1) 
2023-02-15 20:17:14 ✔  $ git diff | cat

diff --git a/src/canonicaljson/__init__.py b/src/canonicaljson/__init__.py
index ef33332..6f27b99 100644
--- a/src/canonicaljson/__init__.py
+++ b/src/canonicaljson/__init__.py
@@ -52,9 +52,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
 
 
 class JsonLibrary(Protocol):  # pragma: no cover
-    @property
-    def JSONEncoder(self) -> Type[Encoder]:
-        pass
+    JSONEncoder: Type[Encoder]

 
dmr on titan in python-canonicaljson on  dmr/typing-jsonmodule via 🐍 v3.11.1 (python-3.11.1) 
2023-02-15 20:17:54 ✔  $ mypy
src/canonicaljson/__init__.py:146: error: Argument 1 to "set_json_library" has incompatible type Module; expected "JsonLibrary"  [arg-type]
src/canonicaljson/__init__.py:146: note: Following member(s) of "Module json" have conflicts:
src/canonicaljson/__init__.py:146: note:     JSONEncoder: expected "Type[Encoder]", got "Type[JSONEncoder]"
tests/test_canonicaljson.py:169: error: Argument 1 to "set_json_library" has incompatible type Module; expected "JsonLibrary"  [arg-type]
tests/test_canonicaljson.py:169: note: Following member(s) of "Module json" have conflicts:
tests/test_canonicaljson.py:169: note:     JSONEncoder: expected "Type[Encoder]", got "Type[JSONEncoder]"
Found 2 errors in 2 files (checked 3 source files)

@DMRobertson DMRobertson merged commit ebab5ec into main Feb 15, 2023
@DMRobertson DMRobertson deleted the dmr/typing-jsonmodule branch February 15, 2023 21:35
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request Feb 15, 2023
Tweak DBAPI2 Protocol to be accepted by mypy 1.0
d695914 
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
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request Feb 15, 2023
Tweak DBAPI2 Protocol to be accepted by mypy 1.0
e9329dc 
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
DMRobertson pushed a commit to matrix-org/synapse that referenced this pull request Feb 16, 2023
Use mypy 1.0 (#15052)
ffc2ee5 
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DMRobertson @clokep