python: consider output header w/ expandable width #7053

chu11 · 2025-09-12T18:17:03Z

Problem: The expandable field prefix ("+:") does not consider the width of the output header name when calculated output width.
If all output entries are less than the size of the output header name, then output will not look as expected.

Solution: When calculating maximum output width of a field, take into account the header entry name.

Fixes #7030

Edit: Oops, some python tests I forgot to update, working on em :-)

grondo · 2025-09-12T19:30:47Z

src/bindings/python/flux/util.py

+                initialmaxwidth = spec.width or 0
+                if (
+                    sentinels[end] in ("maxwidth", "both")
+                    and self.headings
+                    and self.headings[field]
+                    and len(self.headings[field]) > initialmaxwidth
+                ):
+                    initialmaxwidth = len(self.headings[field])


Unfortunately, this applies the heading length to maxwidth even if -n, --no-header is used, which isn't what we want.

Let me ponder the best way to fix this. It may be to just pass args.no_header into the initializer then zero out the headings (if that works, it may not be so simple for JobInfoFormatter)

Yeah, I did realize that. We seem to always pass headers into OutputFormat.

I did consider it being a possibly separate issue. Notice this commit I just added 0e4a22e. That's just to get a test to pass, checking if headers exist in other parts of OutputFormat probably also warranted.

Yeah, and the design allows headers to be class variables when you subclass so just not passing in headings is not a great solution.

The simplest thing would be just add a no_headers=False optional argument to the class initializer and use this in the test above. Unfortunately we'll have to update all callers, but I don't really see a cleaner way around it for now.

Finally, to make this code cleaner, the class should start with an empty self.headings, e.g. self.headings = {}. Then you can drop the test for self.headings. Then avoid accessing self.headings[field] multiple times by doing something like:

if sentinels[end] in ("maxwidth", "both"): initialmaxwidth = max(spec.width or 0, len(self.headings.get(field, "")))

The simplest thing would be just add a no_headers=False optional argument to the class initializer and use this in the test above. Unfortunately we'll have to update all callers, but I don't really see a cleaner way around it for now.

Hmmm. Technically I think we only have to do this for JobInfoFormat? B/c w/ OutputFormat we can just pass in or not pass in a headings. For JobInfoFormat we are always setting one. Or are you thinking of doing both for just consistency?

Since the code to set the maxwidth is in the base OutputFormat class, it should take no_header in its initializer I'd think.

The initialization of self.headings in OutputFormat to and empty dictionary would just seem to make the rest of the code cleaner because you would not have to check for self.headings = None anywhere.

Let me check if that would mess up any other assumptions though.

I think it could be in one PR since it is only required to fix this issue properly.

Hmmmm. I just noticed that the primary user of the filter() function in OutputFormat is the print_items() function, which already has a no_header flag. So we could get away with adding no_header to filter() instead. Any objections? That would take care of the majority of cases, just a few oddball ones (for example flux-jobs calls filter() directly).

Edit: As I look at this more, I'm liking this option more

Yeah, that is what I was envisioning as the easiest "fix".

ahh, you mentioned passing no_header in the initializer at first, so that was what I was going with initially.

Oh yeah, I did, didn't I? Sorry, I probably shouldn't mention specifics until actually looking at the implementation again 🤦

src/cmd/flux-pgrep.py

chu11 · 2025-09-16T19:17:07Z

re-pushed with fixes per discussion above

grondo

This turned out excellent. Nice work!

Problem: There is no documentation for the args to the OutputFormat filter() function. Add inline documentation.

Problem: In the future the header may be needed in calculations within the OutputFormat filter() function. There is no way to inform the filter() function that the header should not be considered (for example, if the header fill not be output). Add a no_header flag to the filter() function in OutputFormat(). This parameter currently does nothing. Update all callers as needed.

Problem: The expandable field prefix ("+:") does not consider the width of the output header name when calculating output width. If all output entries are less than the size of the output header name, then output will not look as expected. Solution: When calculating maximum output width of a field, take into account the header entry name as long as no_header is not set. Update some tests in python/t0024-util.py for changes. Fixes flux-framework#7030

Problem: Tests already exist for flux-jobs expandable fields ("+:") but do not cover the special case when all outputs are less than the length of the column header. Add a test in t2800-jobs-cmd.t.

Problem: The current OutputFormat python tests always assume output headers used in expandable width tests. Add some tests that set no_headers=True, so that the output headers are not taken into account.

codecov · 2025-09-16T22:33:09Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.08%. Comparing base (3776faf) to head (8c72102).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7053   +/-   ##
=======================================
  Coverage   84.08%   84.08%           
=======================================
  Files         549      549           
  Lines       92521    92524    +3     
=======================================
+ Hits        77796    77802    +6     
+ Misses      14725    14722    -3

Files with missing lines	Coverage Δ
src/bindings/python/flux/util.py	`96.64% <100.00%> (+0.01%)`	⬆️
src/cmd/flux-jobs.py	`96.68% <100.00%> (ø)`
src/cmd/flux-pgrep.py	`95.12% <100.00%> (ø)`

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

grondo reviewed Sep 12, 2025

View reviewed changes

chu11 force-pushed the issue7030_flux_jobs_column_width branch 2 times, most recently from f9bf273 to 0743b25 Compare September 16, 2025 17:17

github-advanced-security bot found potential problems Sep 16, 2025

View reviewed changes

src/cmd/flux-pgrep.py Fixed Show fixed Hide fixed

chu11 force-pushed the issue7030_flux_jobs_column_width branch from 0743b25 to ec16919 Compare September 16, 2025 17:36

github-advanced-security bot found potential problems Sep 16, 2025

View reviewed changes

src/cmd/flux-pgrep.py Dismissed Show dismissed Hide dismissed

chu11 force-pushed the issue7030_flux_jobs_column_width branch 2 times, most recently from 0eb078f to 1f82640 Compare September 16, 2025 19:16

grondo approved these changes Sep 16, 2025

View reviewed changes

chu11 mentioned this pull request Sep 16, 2025

flux-jobs: support W presentation type, update cute output format #5179

Open

chu11 added 5 commits September 16, 2025 15:03

python: doc args to OutputFormat filter()

859ec72

Problem: There is no documentation for the args to the OutputFormat filter() function. Add inline documentation.

t: cover flux-jobs expandable field headers

2cbdb9b

Problem: Tests already exist for flux-jobs expandable fields ("+:") but do not cover the special case when all outputs are less than the length of the column header. Add a test in t2800-jobs-cmd.t.

t/python: add extra OutputFormat coverage

8c72102

Problem: The current OutputFormat python tests always assume output headers used in expandable width tests. Add some tests that set no_headers=True, so that the output headers are not taken into account.

chu11 force-pushed the issue7030_flux_jobs_column_width branch from 1f82640 to 8c72102 Compare September 16, 2025 22:04

chu11 added the merge-when-passing label Sep 16, 2025

mergify bot merged commit a29d29d into flux-framework:master Sep 16, 2025
35 checks passed

chu11 deleted the issue7030_flux_jobs_column_width branch September 17, 2025 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

python: consider output header w/ expandable width #7053

python: consider output header w/ expandable width #7053

Uh oh!

chu11 commented Sep 12, 2025 •

edited

Loading

Uh oh!

grondo Sep 12, 2025

Uh oh!

chu11 Sep 12, 2025

Uh oh!

grondo Sep 12, 2025 •

edited

Loading

Uh oh!

chu11 Sep 12, 2025

Uh oh!

grondo Sep 12, 2025

Uh oh!

grondo Sep 12, 2025

Uh oh!

chu11 Sep 15, 2025 •

edited

Loading

Uh oh!

grondo Sep 15, 2025

Uh oh!

chu11 Sep 15, 2025

Uh oh!

grondo Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

chu11 commented Sep 16, 2025

Uh oh!

grondo left a comment

Uh oh!

Uh oh!

codecov bot commented Sep 16, 2025

Uh oh!

Uh oh!

python: consider output header w/ expandable width #7053

python: consider output header w/ expandable width #7053

Uh oh!

Conversation

chu11 commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grondo Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chu11 Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chu11 commented Sep 16, 2025

Uh oh!

grondo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Sep 16, 2025

Codecov Report

Uh oh!

Uh oh!

chu11 commented Sep 12, 2025 •

edited

Loading

grondo Sep 12, 2025 •

edited

Loading

chu11 Sep 15, 2025 •

edited

Loading