Sydent typing, part 2 by DMRobertson · Pull Request #1124 · matrix-org/matrix.org

DMRobertson · 2021-11-26T13:26:11Z

Second part of #1110. Contains a forward and backref to #1123 and #1125.

Tentatively scheduled for Dec 10th.

Preview: https://pr1124--matrix-org-previews.netlify.app/blog/2021/12/10/type-coverage-for-sydent-annotation

Preview: https://pr1124--matrix-org-previews.netlify.app

squahtx

spotted some typos

H-Shay · 2021-12-01T20:51:46Z

+
+Twisted makes use of `zope`'s `Interface` to define a number of abstract interface classes. Speaking personally, I've not seen it used outside twisted, and I think that means it's not supported by much of the typechecking tooling. For example, I've definitely seen PyCharm struggle to realise that it's okay to pass a `Response` to a function which expects an `IResponse`! Here's a more complicated example where `PyCharm` isn't happy with me widening the type `LoggingHostnameEndpoint` to `IStreamClientEndpoint`, even though [the latter implements the former](https://github.com/matrix-org/sydent/blob/92ff7a878a25696365b10cc49e32f5cba32c5960/sydent/http/matrixfederationagent.py#L379-L380).
+
+![Screenshot from pycharm showing a false positive warning](https://i.imgur.com/27rqjHm.png)


I am not sure if this matters but the screenshot is not displaying on the preview, thought I would mention it!

Thanks. Hopefully fixed with 07dfe91 and 56dfb03.

H-Shay · 2021-12-01T21:04:23Z

I spotted one or two typos but otherwise outstanding job! Really informative and clear, which isn't that easy to do!

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

DMRobertson · 2021-12-06T15:44:03Z

Rebased on top of master. I've added links for part 1 -> 2 and removed links 2 -> 3.

callahad · 2021-12-08T19:04:41Z

+
+Picking and choosing easy targets works fairly well, but sometimes that means you end fixing an error that's really a symptom of an earlier one. Other times fixing one error, e.g. by giving a return type annotation to a function—would solve a series of other errors throughout the file. Watching the total number of errors mypy reports bob up and down was intriguing!
+
+In retrospect, I think it would be smoother to generate some kind of dependency graph for the package. I'm imagining a [DAG](https://en.wikipedia.org/wiki/Directed_acyclic_graph) where whose vertices are modules, and there's an edge `A -> B` if `A` imports from `B`. The leaves of this DAG are the ideal place to start: you can get something typechecked there without having to annotate a long chain of dependencies. Another strategy would be to see which modules were the least precise according to mypy's reports—but more on those [next time](2021-12-03-sydent-typing-1.mdx).


The "next time" link 404's

and it actually links to the previous time!

Should be fixed in a7185f8

callahad · 2021-12-08T20:21:28Z

+- That will later be resolved by twisted to an `Any` value. Twisted will send that value to `request`, and the execution continues.
+- This repeats, until we eventually return an `IResponse`.
+
+(I'm guessing `x = await y` is really some kind of sugar around this mechanism: first `yield y`, then the machinery running your coroutine `c` will call `c.send(x)` to resume execution.)


This seems like it's referencing a code snippet that I haven't seen yet?

I've tried to improve this in 54ac758. What do you think @callahad ?

callahad · 2021-12-08T20:26:51Z

+#### You're at the mercy of your dependencies
+
+Hmm. That's being a little melodramatic.
+
+Improving coverage boils down to giving the typechecker more information about your program. The more information it has, the more it can check—and the more errors it can spot. (Hopefully this doesn't make typing come across like a pyramid scheme.) If your dependencies aren't typed, mypy can't validate you're correctly providing inputs and correctly consuming outputs. You might have a bigger impact on overall typing coverage by annotating a dependency (directly or via stubs). I have a hunch that bugs are more likely in code that uses an external dependency: we're much more familiar with the details of our own source code compared to that of a third party we trust.
+
+It's worth looking to see if your dependencies have a newer version including type annotations. Failing that, they may have a stub package added to [`typeshed`](https://github.com/python/typeshed/tree/master/stubs) and published on PyPI. I saw example of both cases when choosing how to configure mypy for Sydent. If some of your dependencies are under your control, consider annotating them—you'll feel the benefits across multiple projects pretty quickly.


This feels like it should be an H3 and come before the "Annotations when working with twisted" heading. With that shuffled up, you'd end up with an outline more like "What have we learned? Well, we're at the mercy of our dependencies. Also, we had to do a lot of work annotating Twisted. Now, here are some tricks learned along the way..."

Thanks, I've had a go at this in 66d04b6.

callahad · 2021-12-08T20:35:38Z

+
+#### [TypedDict](https://docs.python.org/3/library/typing.html?highlight=typeddict#typing.TypedDict)
+
+I mentioned this earlier when talking about the missing `await` bug.


s/earlier/in last week's blog post/

(with link?)

Thanks, this is 6b4910f

Co-authored-by: Dan Callahan <danc@element.io>

because our CSS doesn't make it obvious these are links.

MadLittleMods

Really nice read @DMRobertson 😀 and lots of good learnings!

Co-authored-by: Eric Eastwood <erice@element.io>

It's odd to have roots plural, and confusing to use tree/forest terminology. Plus it's not clear if edges in a tree point to children or parents. Instead, use "sink".

This was referenced Nov 26, 2021

Sydent typing, part 3 #1125

Merged

Sydent typing, part 1 #1123

Merged

squahtx reviewed Nov 26, 2021

View reviewed changes

H-Shay reviewed Dec 1, 2021

View reviewed changes

Comment thread gatsby/content/blog/2021/12/2021-12-03-sydent-typing-1.mdx Outdated

H-Shay reviewed Dec 1, 2021

View reviewed changes

Comment thread gatsby/content/blog/2021/12/2021-12-10-sydent-typing-2.mdx Outdated

H-Shay reviewed Dec 1, 2021

View reviewed changes

David Robertson and others added 5 commits December 6, 2021 15:22

Sydent typing, part 2

e558ce9

Typo fixes, thanks Sean!

da0810f

Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com>

Remove redundant paragraph

ca61ce5

Fix image link (whoops)

07dfe91

Add links to part 2 from part 1

56b30d1

DMRobertson force-pushed the dmr/sydent-typing-2 branch from 0c95710 to 56b30d1 Compare December 6, 2021 15:37

Remove links to part 3 from part 2

a008828

David Robertson added 2 commits December 6, 2021 15:46

Remove thoughts category

d77de49

Fix image URL: 2012 -> 2021

56dfb03

callahad reviewed Dec 8, 2021

View reviewed changes

Remove incorrect and premature "next time" link

a7185f8

callahad reviewed Dec 8, 2021

View reviewed changes

Comment thread gatsby/content/blog/2021/12/2021-12-10-sydent-typing-2.mdx Outdated

callahad reviewed Dec 8, 2021

View reviewed changes

Comment thread gatsby/content/blog/2021/12/2021-12-10-sydent-typing-2.mdx Outdated

callahad reviewed Dec 8, 2021

View reviewed changes

David Robertson and others added 5 commits December 9, 2021 12:16

object <-> Any

fb8bab9

Co-authored-by: Dan Callahan <danc@element.io>

Don't use <code> inside <a>nchors

28cf20a

because our CSS doesn't make it obvious these are links.

Try to clarify the async/await generator stuff

54ac758

Add backlink to part 1

6b4910f

Rearrange "mercy of dependencies" bit

66d04b6

MadLittleMods requested changes Dec 10, 2021

View reviewed changes

Comment thread gatsby/content/blog/2021/12/2021-12-10-sydent-typing-2.mdx Outdated

David Robertson and others added 2 commits December 10, 2021 12:13

end -> end up, thanks Eric

ac3fc2f

Co-authored-by: Eric Eastwood <erice@element.io>

Clarify "roots" of DAG

c171447

It's odd to have roots plural, and confusing to use tree/forest terminology. Plus it's not clear if edges in a tree point to children or parents. Instead, use "sink".

DMRobertson merged commit ff92f85 into master Dec 10, 2021

DMRobertson deleted the dmr/sydent-typing-2 branch December 10, 2021 12:27

rumpelsepp mentioned this pull request Mar 14, 2023

Update mnotify rumpelsepp/matrix.org#1

Closed


		Twisted makes use of `zope`'s `Interface` to define a number of abstract interface classes. Speaking personally, I've not seen it used outside twisted, and I think that means it's not supported by much of the typechecking tooling. For example, I've definitely seen PyCharm struggle to realise that it's okay to pass a `Response` to a function which expects an `IResponse`! Here's a more complicated example where `PyCharm` isn't happy with me widening the type `LoggingHostnameEndpoint` to `IStreamClientEndpoint`, even though [the latter implements the former](https://github.com/matrix-org/sydent/blob/92ff7a878a25696365b10cc49e32f5cba32c5960/sydent/http/matrixfederationagent.py#L379-L380).

		![Screenshot from pycharm showing a false positive warning](https://i.imgur.com/27rqjHm.png)


		Picking and choosing easy targets works fairly well, but sometimes that means you end fixing an error that's really a symptom of an earlier one. Other times fixing one error, e.g. by giving a return type annotation to a function—would solve a series of other errors throughout the file. Watching the total number of errors mypy reports bob up and down was intriguing!

		In retrospect, I think it would be smoother to generate some kind of dependency graph for the package. I'm imagining a [DAG](https://en.wikipedia.org/wiki/Directed_acyclic_graph) where whose vertices are modules, and there's an edge `A -> B` if `A` imports from `B`. The leaves of this DAG are the ideal place to start: you can get something typechecked there without having to annotate a long chain of dependencies. Another strategy would be to see which modules were the least precise according to mypy's reports—but more on those [next time](2021-12-03-sydent-typing-1.mdx).


		#### [TypedDict](https://docs.python.org/3/library/typing.html?highlight=typeddict#typing.TypedDict)

		I mentioned this earlier when talking about the missing `await` bug.

Uh oh!

Conversation

DMRobertson commented Nov 26, 2021 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

squahtx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DMRobertson Dec 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

H-Shay commented Dec 1, 2021

Uh oh!

DMRobertson commented Dec 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MadLittleMods left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DMRobertson commented Nov 26, 2021 •

edited by github-actions Bot

Loading

DMRobertson Dec 6, 2021 •

edited

Loading