-
Notifications
You must be signed in to change notification settings - Fork 87
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
[TD] Historical edited files and profiling heuristics #4590
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per offline convo, could you please split this PR up into two: One which adds the new heuristic data and one that adds all the other refactorings?
torchci/scripts/utils.py
Outdated
from typing import Any, Dict, List, Optional, Union | ||
import requests | ||
|
||
import rockset # type: ignore[import] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a generic utils file inside the torchci/scripts folder, I'd suggest being picky about what exactly goes inside here so that this file doesn't get too bloated over time.
General utils could be kept here, but the rockset and heuristic utils are specialized enough to warrant their own files.
I do like the idea of a the naming convention though. What if those files were all prefixed with "utils_", so it became utils_rockset.py
and utils_heuristics.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i moved the functions out to their own files, but they seem a bit short haha
I also named it with utils in front for the files that are new
@@ -0,0 +1,61 @@ | |||
from collections import defaultdict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about putting these td_
scripts inside a td
subfolder instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly a future thing, but also weird python module import problems make this annoying
) | ||
|
||
|
||
def cache_json(func): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
Add a heuristic based on profiling data and a heuristic based on historical edited files.
Addes caching for rockset queries and other functions.