Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add type hints to synmark. #16421

Merged
merged 10 commits into from
Oct 4, 2023
Merged

Add type hints to synmark. #16421

merged 10 commits into from
Oct 4, 2023

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 2, 2023

Was trying to figure out synmark the other day and type hints were helpful.

@clokep clokep marked this pull request as ready for review October 3, 2023 11:12
@clokep clokep requested a review from a team as a code owner October 3, 2023 11:12
synmark/__main__.py Outdated Show resolved Hide resolved

d.addBoth(on_done)
reactor.callWhenRunning(lambda: d.callback(True))
reactor.run()

return d.result
return cast(float, d.result)
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised a cast is needed here. Any idea why?

Copy link
Member Author

Choose a reason for hiding this comment

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

synmark/__main__.py:61: error: Incompatible return value type (got "object", expected "float")  [return-value]

I think the Deferred isn't being properly marked as returning a float? (Maybe the Twisted trunk fixes this?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't quite figure this out, I think twisted trunk would fix it though.

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

Apart from #16421 (comment) though this seems sane!

@clokep clokep requested a review from DMRobertson October 4, 2023 17:23
print(file_out.getvalue())
reactor.stop()
return _
return res

d.addBoth(on_done)
reactor.callWhenRunning(lambda: d.callback(True))
Copy link
Contributor

Choose a reason for hiding this comment

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

Surprised mypy doesn't complain about feeding a bool into a Deferred[float]!

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

I'm happy, provided you've tested that synmark still actually runs!

@clokep
Copy link
Member Author

clokep commented Oct 4, 2023

I have! The logging suite seems broken unrelated to this PR so I Don't want to hold this up.

@clokep clokep merged commit ab9c1e8 into develop Oct 4, 2023
30 checks passed
@clokep clokep deleted the clokep/synmark-types branch October 4, 2023 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants