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

Add an option to specify time format #102

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

eltoder
Copy link
Contributor

@eltoder eltoder commented Nov 19, 2024

str(float) prints 16 decimal places, which is too verbose and much more than the precision in the profiles. Print 7 digits by default and allow setting the format with --time-format.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.70%. Comparing base (4f75c58) to head (cc55157).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
gprof2dot.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   85.76%   85.70%   -0.06%     
==========================================
  Files           1        1              
  Lines        2452     2449       -3     
==========================================
- Hits         2103     2099       -4     
- Misses        349      350       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@eltoder eltoder force-pushed the feature/time-format branch from 24a8507 to 4750704 Compare November 19, 2024 00:12
Copy link
Owner

@jrfonseca jrfonseca left a comment

Choose a reason for hiding this comment

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

Otherwise looks good.

I actually wonder if --time-format option is worthwhile, but have no strong opinion either way.

gprof2dot.py Outdated
@@ -44,6 +44,7 @@


MULTIPLICATION_SIGN = chr(0xd7)
TIME_FORMAT = "%.7g"
Copy link
Owner

Choose a reason for hiding this comment

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

Given TIME_FORMAT is not a constant, but rather a variable, please rename it to timeFormat.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

str(float) prints 16 decimal places, which is too verbose and much more
than the precision in the profiles. Print 7 digits by default and allow
setting the format with --time-format.
@eltoder eltoder force-pushed the feature/time-format branch from 4750704 to cc55157 Compare November 19, 2024 12:09
@eltoder
Copy link
Contributor Author

eltoder commented Nov 19, 2024

Otherwise looks good.

I actually wonder if --time-format option is worthwhile, but have no strong opinion either way.

I think picking a format that pleases everyone can be hard and it's easy to make it configurable, so we can avoid arguing about it.

@eltoder
Copy link
Contributor Author

eltoder commented Nov 21, 2024

@jrfonseca please take another look

btw, --time-format can also be used to address #58 -- if one wants to see units in the labels, they can add it to the format, e.g. --time-format=%.6gs.

@jrfonseca jrfonseca merged commit d5f801c into jrfonseca:master Nov 22, 2024
5 checks passed
@jrfonseca
Copy link
Owner

btw, --time-format can also be used to address #58 -- if one wants to see units in the labels, they can add it to the format, e.g. --time-format=%.6gs.

Good point.

Merged. Thanks.

@eltoder eltoder deleted the feature/time-format branch November 22, 2024 14:07
@eltoder
Copy link
Contributor Author

eltoder commented Nov 22, 2024

Thank you

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