Number formatting: thousands separators and numeric alignment - #52
Open
derekwisong wants to merge 11 commits into
Open
Number formatting: thousands separators and numeric alignment#52derekwisong wants to merge 11 commits into
derekwisong wants to merge 11 commits into
Conversation
Digit grouping (thousands separators), decimal separator choice, and optional fixed float precision, with named presets covering the common locale conventions. No callers yet. Formatting is display-only and never feeds back into Polars. The hot path runs once per visible cell per frame, so it is allocation free beyond the destination String the caller already needs: integers are written digit-by-digit into a stack buffer with separators emitted inline, and width_i64 computes display width arithmetically so the locked-column measurement pass need not build a string it discards. Floats with no explicit precision are regrouped from Polars' own rendering, so toggling formatting never changes how many decimal places a value shows. Benchmarks over a 1000-cell frame: the formatted path costs ~25us against ~56us for the current unformatted AnyValue::str_value() path, and the arithmetic width pass ~2.8us. Formatting numerics is cheaper than not formatting them because it skips Polars' fmt machinery. Locale conventions are exposed as explicit presets rather than detected from the environment: the data is locale-neutral, LC_NUMERIC is unset or C on much of the target audience's infrastructure, and ICU/CLDR data is multiple megabytes for a single-binary TUI. preset_for_locale_tag backs an opt-in "system" setting for users who do want it.
number_format accepts either a preset name shorthand (number_format = "thousands") or a [display.number_format] table with individual overrides: grouping, separators, min_digits, floats, float_precision, and exclude/include column globs. Presets cover the common locale conventions. grouping = "system" is the one environment-dependent value and it is opt-in, because a data file has no locale and the same file should render identically on every machine. Defaults keep rendering unchanged: grouping is "none", and min_digits is 5 so that years stay 2024 rather than becoming 2,024 once a user does opt in. align_numeric_right defaults to true instead, since alignment changes neither the characters of a value nor a column's width -- values move within space the column already occupied. Bad preset names, multi-character separators, and a group separator equal to the decimal separator are rejected by validate(), which reports the offending config file path, rather than being silently ignored at render time. The generated config is the main discovery surface for the long form, so the field comment shows every preset, the full table, and the F toggle. Blank comment lines are now emitted bare so generated configs carry no trailing whitespace.
Both follow the existing --column-colors pattern: an Option that overrides the corresponding config value in main.rs after load. --number-format uses a clap PossibleValuesParser so bad values are rejected up front with the valid list, rather than surfacing later as a config error. datui-cli cannot depend on datui-lib, so the value list is duplicated there; a test in datui-lib asserts the two stay in sync. The flag overrides only the grouping style via with_grouping_override, preserving exclude_columns, min_digits and precision from a user's [display.number_format] table. Replacing the whole setting would silently discard configuration the user did not ask to change. Regenerates docs/reference/command-line-options.md.
RenderContext carries the resolved NumberFormatSettings through to the DataTable widget, and render_dataframe formats each cell through numfmt. The per-column formatter is resolved once per column, alongside the existing cell_style, so dtype eligibility and the include/exclude globs never run per cell. Columns that need no formatting take the same Cow::Borrowed passthrough the code took before, and a single scratch buffer is reused for the whole frame. The locked-column pass measures widths with numfmt::display_width, which takes an arithmetic path for integers. That pass previously built and discarded a String for every locked cell just to count its characters. Binary columns hold the placeholder stub rather than a number, so they are always passthrough.
Integer and float columns render flush right so magnitudes line up; strings, booleans, temporals and binary stubs stay left. Headers follow their column, since a left-aligned heading over right-aligned digits reads as a rendering bug. Alignment uses Line::right_aligned() rather than padding cells with spaces, which would add an allocation per cell and fight the max-content width calculation. This is on by default (display.align_numeric_right), unlike grouping. Alignment changes neither the characters of a value nor a column's width -- values move within space the column already occupied, so nothing reflows and copied text is unchanged. Grouping does both, which is why it stays opt-in. Numeric columns are never truncated when they overflow (a partial number reads as a wrong number), so alignment can never clip a digit.
Formatting is applied at render time, so the toggle takes effect on the next frame with no re-collect or re-query. Session-only: the config file stays the source of truth at launch. F is a binary on/off rather than a cycle through presets. Choosing which style to use is a config decision made once; turning formatting off is the in-the-moment need, for reading an exact value or copying a coordinate out of the terminal. Uppercase matches the existing display-toggle convention (N for row numbers), and F reads as Format. The comma key would have been a better mnemonic only for the default preset -- a user running "european" has a comma as their decimal point. resolve() now starts formatting enabled only when the user configured something. When they did not, F would otherwise have been a dead key for everyone who never edited their config, so the toggle target becomes Thousands grouping while keeping any other settings they chose. There is deliberately no on-screen indicator: the digits regrouping in place is the feedback. That makes the help overlay the only discovery path for F, so a test now asserts it stays listed there.
The count and null_count columns in the statistics table are row counts that run large on the datasets this feature is for, and they were the only numbers there rendered as raw digits. They now follow the same grouping setting as the data table, so a user who pressed F sees it wherever they read numbers. Float statistics keep going through format_num, which switches to scientific notation at 1000 -- well before grouping would ever apply. Chart axis labels are deliberately left alone. format_axis_label already switches to scientific notation at 1e6, so grouping would only affect the 1,000-999,999 band; axis labels are width-critical and extra characters risk tick overlap; and the function is shared with chart PNG/EPS export, which would pull a display toggle into a file-writing path. The info panel and control bar counts are chrome rather than data and already group unconditionally; they keep doing so.
controls.rs and info.rs each carried their own hand-rolled thousands separator routine, both allocating a Vec<char> or repeatedly inserting at the front of a String. Both now call numfmt::group_chrome. Their output is unchanged and stays unconditional. The control bar's row count and the info panel's totals are datui's own labels rather than the user's data, so they group regardless of display.number_format or the F toggle -- turning formatting off to read an exact data value should not make the surrounding UI harder to read.
Adds a number formatting section to the configuration guide: the preset table, the long form, why min_digits matters, and the display-only boundary. Documents why formatting is not taken from the locale by default, since "why doesn't it follow LC_NUMERIC" is the obvious question, and shows the opt-in for users who do want it. The keyboard reference had no Display toggles section at all -- N was undocumented there before this change. F is discoverable only through the help overlay and this page, so both now list them.
min_digits was a magnitude heuristic for a semantic problem. What it tried to express is "this column holds identifiers, not quantities", but it only caught identifiers that happen to be four digits or fewer. Sample IDs, accession numbers, ZIP codes and PMIDs are mostly longer and sailed straight past it, so exclude_columns was already the mechanism that actually solved the problem and min_digits was a partial duplicate. Its cost was a column rendering "1000" next to "248,956,422". Scoping the threshold per column would not fix that: over the visible window formatting would change as you scroll, over the buffer it would change at boundaries the user cannot see, and over the whole column it would need a full scan -- which is what datui's lazy design exists to avoid, and which Parquet statistics could serve but CSV could not, making behaviour differ by file format. There is no scope that is both stable and cheap, so the threshold goes. Grouping is opt-in, so a user who turns it on can reasonably name the columns that should stay plain. The four-digit exception is a prose convention in any case; in a table, uniform treatment of a column reads better. include_columns existed only to override min_digits and has no meaning without it, so it goes too. exclude_columns remains as the single explicit mechanism, and formatter_for loses a branch.
A misspelled key was silently ignored, and the failure was invisible: every field has a default, so the table resolved to "no formatting" -- exactly what the default config does. A user who wrote groupng instead of grouping saw identical output whether they typo'd it or never wrote the file, with no error and nothing to notice. NumberFormatTable now captures unrecognised keys via serde(flatten) and resolve() names them. Coming through AppConfig::load, the user gets the config file path, the offending key, and the accepted set. deny_unknown_fields would not work here: the setting is an untagged enum so that a preset name and a table are both accepted, and a denied field makes the Custom variant fail to match, leaving serde to report only that no variant matched. Verified that capturing keys this way does not degrade type errors, which still surface as TOML parse errors pointing at the offending line. Also return zero width from write_magnitude's unreachable non-UTF-8 branch. It previously returned the computed width while pushing nothing; callers size table columns from that value, so the impossible case would have corrupted layout rather than failing visibly.
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
1 participant
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.
Closes #51.
Adds digit grouping for numbers in the data table, plus right-aligned numeric columns. Genomic coordinates in a BED file go from
248956422to248,956,422.Using it
or
--number-format thousands, or press F in the table to toggle it at any time.Presets:
none(default),thousands,european,si,swiss,indian,underscore. A[display.number_format]table gives per-field control — separators, float precision, andexclude_columns.Design decisions worth reviewing
Grouping is off by default; alignment is on. Grouping rewrites the characters of a value and widens the column, so it stays opt-in. Alignment changes neither — values move within space the column already occupied, so nothing reflows and copied text is identical.
align_numeric_right = falserestores the old look.Every value in a formatted column is grouped, with no size threshold. An earlier revision had a
min_digitsoption (default 5) so that ayearcolumn stayed2024rather than2,024. It was removed: it was a magnitude heuristic for a semantic problem, catching only identifier columns that happen to be short, while sample IDs and accession numbers sailed past it and neededexclude_columnsanyway. Its cost was a column rendering1000next to248,956,422. No scope for a per-column threshold is both stable and cheap — visible-window flickers on scroll, buffer changes at boundaries the user can't see, and whole-column needs a full scan (available from Parquet statistics but not CSV, so behaviour would differ by format).exclude_columnsis now the single explicit mechanism for keeping a column plain.No automatic locale detection. A data file has no locale, so the same file should render identically on a laptop and over SSH.
LC_NUMERICis also unset orCon much of the infrastructure this is aimed at, so detection would silently do nothing exactly where it was wanted — and real locale support means megabytes of ICU/CLDR data in a single-binary TUI. The presets cover the same conventions explicitly, andgrouping = "system"is available as an opt-in.No on-screen indicator for the F toggle. The digits regrouping in place is the feedback, matching how
N(row numbers) already behaves. That makes the help overlay the only discovery path, so a test assertsFstays listed there.resolve()also starts formatting on only if you configured something — otherwiseFwould have been a dead key for anyone who never edited their config, and the toggle target becomesthousands.Formatting is display-only. Exports, queries, filters, templates and group-by keys always use raw values. The control bar and info panel counts are datui's own chrome and stay grouped unconditionally, so turning formatting off to read an exact value never makes the surrounding UI harder to read.
Performance
Formatting runs once per visible cell per frame. Over a 1000-cell frame:
AnyValue::str_value())Formatting numerics is faster than not formatting them, because it skips Polars'
fmtmachinery. The locked-column pass previously built and discarded aStringper cell just to count characters; it now takes an arithmetic path. A criterion bench (crates/datui-lib/benches/numfmt.rs) pins these so a future allocation in the row loop shows up.The typed
ChunkedArraydowncast I had floated is not included — the bench showedAnyValuedispatch is not the cost.Not included
format_axis_labelalready switches to scientific notation at 1e6, so grouping would only affect the 1,000–999,999 band; labels are width-critical and extra characters risk tick overlap; and the function is shared with chart PNG/EPS export, which would pull a display toggle into a file-writing path.Also in here
Two hand-rolled thousands-separator routines existed already (
controls.rs,info.rs); both now call the shared formatter with unchanged output. The keyboard reference had no Display toggles section at all —Nwas undocumented before this.Test plan
cargo test --workspacegreencargo clippy --workspace --all-targets -- -D warningscleani64::MIN,u64::MAX, Indian grouping, every preset, width-equals-rendered-length across magnitudes, glob matching, locale tag mapping--number-formatfor each preset,Ftoggling on and back off from the default config,--align-numeric-right falseLeft unmerged for review.