gh-154335: Allow disabling terminal colors in Tachyon's pstats_collector module#154344
gh-154335: Allow disabling terminal colors in Tachyon's pstats_collector module#154344pfackeldey wants to merge 5 commits into
pstats_collector module#154344Conversation
…gh environment variables
|
All commit authors signed the Contributor License Agreement. |
|
Maybe this could be extended to include Lib/profiling/sampling/sample.py as well? |
Good point, I missed this one. I'll update the PR asap 👍 |
|
Dear @pablogsal and @edvilme,
I found the fix! the no-color path (on iOS and Android) will not skip the header line of the pstats output in the test case, which it should though to populate diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_profiler.py b/Lib/test/test_profiling/test_sampling_profiler/test_profiler.py
index 2f5a5e27328..6b999e04e3d 100644
--- a/Lib/test/test_profiling/test_sampling_profiler/test_profiler.py
+++ b/Lib/test/test_profiling/test_sampling_profiler/test_profiler.py
@@ -751,6 +751,7 @@ def test_print_sampled_stats_sort_by_name(self):
and not "calls" in line # Skip summary lines
and not "total time" in line # Skip summary lines
and not "cumulative time" in line
+ and not "filename:lineno(function)" in line # Skip header line
): # Skip summary lines
data_lines.append(line)I'm happy to add this if you agree that this is the correct approach. Let me know what you think, Peter |
| import marshal | ||
| import pstats | ||
| lazy from _colorize import ANSIColors | ||
| from _colorize import get_colors |
There was a problem hiding this comment.
I wonder if it would be better to lazily call this import, and instead of reasigning to ANSIColors, we just call get_colors() directly? Or define ANSIColors = get_colors() in places where it will be used
There was a problem hiding this comment.
Sure 👍 I'm happy to change this.
2 participants
This PR uses
_colorize.get_colors()instead of the_colorize.ANSIColorsenum. That way, one can disable terminal coloring through ANSI escape codes with e.g. setting certain env vars, such asNO_COLOR=1.I couldn't find an appropriate place for a test (and also I wasn't sure what's the best way to test this). Please let me know what your preference is regarding a test, and I'm happy to add it.
pstats_collectordoes not respect "no-color" modes #154335