Skip to content

Charts: one combo plot-kind rule; Scale::ticks keeps ticks off the step grid #1146

Open

Description

@vangel76

Part of a crate review of crates/chart (cleanup of duplicate and dead helpers).

  • plot_kind (render.rs), effective_kind and series_plot_kind (resolve.rs) repeat the combo-chart rule, and the fallback differs: a series whose kind is Combo becomes Line when drawing but column (first series) or line (others) when resolving. Resolved series never carry Combo in a combo chart, so the two agree on everything resolve produces; only a hand-built ChartData could tell them apart. They should be one rule.
  • axis::fixed_scale is only a struct literal.
  • Scale::ticks snaps every tick to a multiple of the step, which is wrong for a scale whose min is not on the step grid (for example min 3, max 7, step 4 gives 4 and 8). The public Scale allows such values; nice_scale only produces them in a far fallback.

Activity

  1. vangel76 commented on Oct 13, 2026

    @vangel76
    ContributorAuthor

    I'm working on this.

  2. iluebbe commented on Oct 23, 2026

    @iluebbe

    #1150 checked

    Fixes it. Resolving and drawing now share one combo plot-kind rule, the dead fixed_scale helper is gone, and a scale whose minimum is off the step grid keeps its own ticks.

    Setup: PR head 5cf0e24, merged locally into main at a1c8454, Windows 11. Checked with the chart crate tests and by reading the diff. The PR has since merged main at a1c8454 itself (7f8ac83); that merge was not rebuilt.

    • All three points of the issue are still present on main (render.rs has its own plot_kind, axis.rs has fixed_scale, Scale::ticks snaps every tick to the step grid).
    • Scale { min: 3, max: 7, step: 4 }: main gives ticks 4 and 8, the PR gives 3 and 7. Aligned scales (0 to 1 by 0.1) still snap float error away.
    • drawing_and_resolving_agree_on_the_combo_plot_kind covers every chart kind × every series kind (or none) × series 0..3.
    • The only behaviour change is the one the PR names: a hand-built ChartData with a Combo series kind in a combo chart. resolve never produces that.

    Problems: none found.

    Merging: the checked head conflicted with main at a1c8454 only in crates/chart/src/tests.rs and ROADMAP.md, where both sides append; the updated head 7f8ac83 merges without conflicts. It also overlaps with other open chart PRs:

    Tests: both new tests pass. cargo test -p gridcraft-chart passes (41, merged with main). cargo clippy -p gridcraft-chart --all-targets -- -D warnings and cargo fmt --all -- --check are clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions