-
Notifications
You must be signed in to change notification settings - Fork 305
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
Enhance Benchmark Report Generation #2248
base: main
Are you sure you want to change the base?
Enhance Benchmark Report Generation #2248
Conversation
31f7d63
to
40a66fa
Compare
@@ -39,6 +39,8 @@ jobs: | |||
run: | | |||
invoke benchmark-dtypes | |||
continue-on-error: true |
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.
Should we add a timeout-minutes
to the run_dtypes_benchmark
job?
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.
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.
This looks good! Some minor nitpicks but otherwise good job!
for key, value in comparison_results.items(): | ||
if not value.empty: | ||
sorted_results[key] = value | ||
if key == 'unsupported_dtypes': | ||
slack_messages.append(':fire: New unsupported DTypes!') | ||
mark_results['#EB9999'] = value |
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.
minor: we should label the hex codes as constants like RED_HEX = ...
or whatever it is
# Compute the summary | ||
summary = pd.DataFrame() | ||
for name, df in results.items(): | ||
df = calculate_support_percentage(df) |
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 does this df represent? Can we name it accordingly
@@ -117,6 +133,8 @@ def save_to_gdrive(output_folder, results, output_filename=None): | |||
output_filename (str, optional): | |||
String representing the filename to be used for the results spreadsheet. If None, | |||
uses to the current date and commit as the name. Defaults to None. | |||
mark_results (dict, optional): | |||
A dict mapping to mark the results. |
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.
mapping what to the mark?
Resolves #2235
CU-86b2975ca