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.

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

Type annotations for profile.py #6067

Merged
merged 4 commits into from
Apr 6, 2022
Merged

Conversation

crusaderky
Copy link
Collaborator

Offshoot from #6033

@crusaderky crusaderky self-assigned this Apr 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2022

Unit Test Results

       18 files  +       5         18 suites  +5   8h 50m 24s ⏱️ + 2h 22m 0s
  2 724 tests ±       0    2 642 ✔️ +     12       82 💤  -   12  0 ±0 
24 364 runs  +6 894  23 145 ✔️ +6 538  1 219 💤 +356  0 ±0 

Results for commit 9c46a41. ± Comparison against base commit cd2ce86.

♻️ This comment has been updated with latest results.

Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

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

LGTM. Feel free to merge after resolving merge conflicts and CI is green-ish

@@ -43,7 +44,7 @@
from distributed.utils import color_of


def identifier(frame):
def identifier(frame) -> str:
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why you're never annotating the frame?

Suggested change
def identifier(frame) -> str:
def identifier(frame: types.FrameType) -> str:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added them now

@crusaderky crusaderky merged commit 8cf1196 into dask:main Apr 6, 2022
@crusaderky crusaderky deleted the profile_annotations branch April 6, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants