Skip to content

Let show_components override display units for Vector-form columns#595

Merged
jd-lara merged 2 commits into
IS4from
lk/show-components-units-override
Jul 10, 2026
Merged

Let show_components override display units for Vector-form columns#595
jd-lara merged 2 commits into
IS4from
lk/show-components-units-override

Conversation

@luke-kiernan

Copy link
Copy Markdown
Contributor

Summary

  • Adds an IS.unitful_variant(f) trait that resolves a getter's unit-tagged _unitful companion by the struct-generator's naming convention (falls back to f when no companion is registered).
  • Threads an optional units kwarg through show_components/_resolve_column_accessors so callers can force every Vector-form column into one unit system instead of resolving each column's own display_units_arg default.
  • Vector-form columns whose getter has a units trait now route through unitful_variant so cells print with an explicit unit suffix (e.g. "5 SU"), matching the Dict-form behavior domain packages already build with closures.

Motivation

PowerSystems (psy6, see the units-management rework) needed a show_components(sys, T, [:field, ...]; units = MW) API that both (a) forces a caller-chosen unit system across all columns and (b) prints unit-tagged values, not bare numbers. Neither capability existed on IS's Vector-column path, so PSY reimplemented ~35 lines of show_components's table-building logic (column labels, the comps/data loop, the pretty_table call) just to get them. This PR moves both capabilities into IS so PSY (and other domain packages) can go back to a thin forwarding wrapper.

Test plan

  • julia --project=test test/runtests.jl (IS): 8672 tests, 0 failures
  • New test/test_printing.jl testset covers: trait-default resolution with unit suffix, explicit units override, Vector column order preservation, units silently ignored for Dict-form columns
  • Downstream (PowerSystems, pinned via [sources] to this branch): test/runtests.jl test_printing — 74 passed, 0 failed (1 unrelated pre-existing PowerSystemCaseBuilder data-build failure, reproduced identically against IS4 directly)

🤖 Generated with Claude Code

luke-kiernan and others added 2 commits July 7, 2026 13:19
Adds a unitful_variant trait that resolves a getter's unit-tagged `_unitful`
companion by the struct-generator's naming convention, and threads an optional
`units` kwarg through show_components/_resolve_column_accessors so callers can
force every Vector-form column into one unit system instead of each column's
own display_units_arg default. This lets PowerSystems stop reimplementing the
Vector-column table-building path just to get unit-suffixed cells and a
units override.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
luke-kiernan added a commit to Sienna-Platform/PowerSystems.jl that referenced this pull request Jul 7, 2026
IS (Sienna-Platform/InfrastructureSystems.jl#595) now supports a `units`
override and unit-tagged column resolution on its Vector-column
show_components path, via a new unitful_variant trait. Drop PSY's
reimplementation of that table-building logic and the local
_unitful_getter/_column_accessor helpers in favor of IS.unitful_variant and a
thin forwarding wrapper.

[sources] is pinned to the IS branch backing #595 for development; restore to
IS4 once that PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.99%. Comparing base (fd99539) to head (0289b71).
⚠️ Report is 4 commits behind head on IS4.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              IS4     #595      +/-   ##
==========================================
+ Coverage   83.85%   83.99%   +0.13%     
==========================================
  Files          73       73              
  Lines        6301     6334      +33     
==========================================
+ Hits         5284     5320      +36     
+ Misses       1017     1014       -3     
Flag Coverage Δ
unittests 83.99% <100.00%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/InfrastructureSystems.jl 44.44% <ø> (ø)
src/relative_units.jl 80.82% <100.00%> (+1.11%) ⬆️
src/utils/print_pt.jl 92.25% <100.00%> (+2.19%) ⬆️
src/utils/test.jl 65.00% <100.00%> (+1.84%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances show_components so Vector-form additional_columns can (1) be forced into a caller-selected unit system via a new units keyword and (2) display unit-tagged values by routing unit-aware getters through a _unitful companion resolved by a new unitful_variant trait. This closes a gap versus Dict-form columns (closures) and enables downstream packages (e.g., PowerSystems) to avoid reimplementing table-building logic.

Changes:

  • Added RelativeUnits.unitful_variant(f) to resolve a getter’s *_unitful companion (fallbacks to f when absent).
  • Threaded units through show_components and _resolve_column_accessors so Vector-form columns can be uniformly overridden without forcing a units argument onto non-unit-aware getters.
  • Added tests covering default trait behavior, explicit units override, column-order preservation, and “ignored for Dict-form” behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/test_printing.jl Adds coverage for Vector-form column unit resolution and units override behavior in show_components.
src/utils/test.jl Extends test component getters with a display_units_arg trait and a _unitful companion to exercise the new printing path.
src/utils/print_pt.jl Implements units keyword plumbing and routes unit-aware Vector columns through unitful_variant for unit-suffixed cell output.
src/relative_units.jl Introduces and exports unitful_variant in RelativeUnits.
src/InfrastructureSystems.jl Brings unitful_variant into the InfrastructureSystems namespace via using .RelativeUnits:.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@luke-kiernan

Copy link
Copy Markdown
Contributor Author

Addresses PSY issue #1128: companion of PSY PR #1711.

jd-lara added a commit to Sienna-Platform/PowerSystems.jl that referenced this pull request Jul 10, 2026
…ay in device base (#1711)

* Show explicit unit suffixes for converted fields; rating fields display in device base

Implements the display convention proposed in #1128: unattached components
show needs_conversion fields in natural units, attached components show
system base, and rating/rating_primary/rating_secondary/rating_tertiary
fields always show device base regardless of attachment (a "rating" isn't
scaled by the system the device happens to sit on). Every converted value
now prints with an explicit unit suffix (e.g. "30.0 MW", "1.0 DU", "0.3 SU")
by displaying the getter's `_unitful` companion instead of a bare number.

Also adds a `units` keyword to the new `show_component` function and to
`show_components` to force a specific display unit system per call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* code review

* Delegate show_components Vector-form columns to IS

IS (Sienna-Platform/InfrastructureSystems.jl#595) now supports a `units`
override and unit-tagged column resolution on its Vector-column
show_components path, via a new unitful_variant trait. Drop PSY's
reimplementation of that table-building logic and the local
_unitful_getter/_column_accessor helpers in favor of IS.unitful_variant and a
thin forwarding wrapper.

[sources] is pinned to the IS branch backing #595 for development; restore to
IS4 once that PR merges.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* Pin IS [sources] to a commit SHA instead of a branch name

A branch name under [sources] can move underneath this PR (force-push,
new commits) without CI or reviewers noticing; a SHA is immutable, so the
pin only changes when this PR is explicitly updated to track IS PR #595's
latest commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: jd-lara <jdlara@berkeley.edu>
@jd-lara
jd-lara merged commit c63d9a2 into IS4 Jul 10, 2026
7 of 10 checks passed
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.

Discrepancy between pretty print and getter functions of unattached components

3 participants