Merge CLI formatting stacks: Rich becomes the sole human renderer (#100)#106
Merged
Conversation
Introduce cli/presentation.py owning all CLI data-shaping (field selection, labels, units, ordering, value formatting and energy aggregation) as presentation-neutral structures. The plain-text/CSV/JSON renderer and the Rich renderer now consume these structures instead of re-deriving the same values, removing the duplicated device status/info row building and monthly/daily energy aggregation. CLI output is unchanged for existing commands.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the CLI formatting subsystem to eliminate duplicated “data-shaping” logic across the plain-text/CSV/JSON renderer and the Rich renderer by introducing a presentation-neutral intermediate layer that both output modes consume.
Changes:
- Added
cli/presentation.pyto centralize CLI data-shaping into neutral row structures for device status/info and neutral dataclass reports for energy aggregation. - Updated
output_formatters.pyto render from the neutral structures, removing duplicated aggregation and row-building logic. - Documented the refactor in
CHANGELOG.rstunder “Unreleased”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/nwp500/cli/presentation.py | New presentation-neutral builders for device/status rows and monthly/daily energy report aggregation. |
| src/nwp500/cli/output_formatters.py | Plain-text/CSV/JSON rendering updated to consume the neutral builders; removes duplicated formatting/aggregation logic. |
| CHANGELOG.rst | Adds an “Unreleased” changelog entry describing the CLI formatting architecture change. |
The CLI hard-requires rich (no fallback), so the plain-text human renderer was redundant. Remove the rich-vs-plain fallback machinery from rich_output.py (_should_use_rich, _rich_available, NWP500_NO_RICH, all _print_*_plain methods) and make rich the sole human renderer that consumes the neutral presentation layer directly (EnergyReport/EnergyPeriodRow instead of dict adapters). Rich now renders the energy TOTAL SUMMARY too, so removing the plain-text summary loses no information. Drop the now-dead plain-text energy builders and dict adapters from output_formatters.py, which retains only JSON/CSV rendering plus thin human-output dispatch. Net CLI code drops from 2088 to 1838 lines with a single data-shaping layer (presentation.py) and a single human renderer (rich_output.py). Non-energy output verified byte-identical via golden capture; energy now renders summary + breakdown entirely in rich instead of doubling plain text and a rich table.
eman
changed the title
- Update presentation.py module docstring: the plain-text renderer was removed and Rich is now mandatory, so drop the stale reference. - Derive year/month for print_daily_energy_table() from DailyEnergyReport instead of passing them separately, removing the duplicated source of truth and possible argument drift.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Properly merges the two CLI formatting stacks. Because the CLI hard-requires
rich(no plain-text fallback), the redundant plain-text human renderer and the rich-vs-plain fallback machinery are removed, leaving a single data-shaping layer feeding a single Rich renderer.What changed vs the first attempt
The initial version added
presentation.pybut leftrich_output.pyuntouched with its own internal plain/rich fallback, so total CLI code grew. This revision finishes the job:rich_output.py: removed all fallback machinery —_should_use_rich,_rich_available, thetry/except ImportErrorshim, theNWP500_NO_RICHtoggle, and every_print_*_plainmethod. Rich imports are now unconditional; each public method renders with Rich directly. The energy renderers consume the neutralEnergyReport/EnergyPeriodRow/DailyEnergyReportdataclasses directly (no dict adapters) and now render theTOTAL SUMMARYin Rich, so no information is lost.output_formatters.py: deleted the dead plain-text energy builders (format_energy_usage,format_daily_energy_usage,_format_total_summary) and the_month_rows_to_dicts/_day_rows_to_dictsadapters. It now holds only JSON/CSV rendering plus thin human-output dispatch.presentation.py: unchanged — the single data-shaping layer.Result
Validation
scripts/lint.py(ruff + pyright): passedmypy src/nwp500: no issues in 50 filespytest: 611 passedCloses #100