Cleanup: generate axis ticks in-range instead of pruning after the fact (follow-up to #634)

Abierto
  • ¿Algo no está bien?
#997 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
48/100
Tipo de issue
Refactorización
Claridad
Bastante claro
Estado de actividad
Tranquilo
Stack tecnológico
java
Área
data-visualization

Línea de trabajo

Comienza con AxisTickCalculator_.calculate(), AxisTickLabels y PlotSurface_AxesChart; después, inspecciona PR #996 y RegressionIssue634Test. Compara los enfoques de clipping y de generación de valores dentro del rango, preservando la densidad de ticks existente siempre que sea posible. Se considera terminado cuando la prueba de regresión pasa, el espaciado de la galería permanece sin cambios y no se llama a los formateadores para valores fuera de rango.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Follow-up / tech-debt cleanup from the fix for #634 (PR #996).

Context

#634 was a negative Y-axis label appearing with all-positive data. Root cause: AxisTickCalculator_.calculate() deliberately overshoots the tick range — getFirstPosition() starts one grid step below minValue and the loop runs to maxValue + 2*gridStep — and the tick-label renderer (AxisTickLabels) clips only to the taller axis-column bounds, so overshoot labels leak into the plot margin. (Gridlines and tick marks already clip correctly to the plot bounds in PlotSurface_AxesChart.)

PR #996 fixed the visible bug by pruning ticks outside the data band [margin, margin+tickSpace] after the generation do-while settles. That was chosen deliberately to be behavior-preserving: pruning post-loop leaves the grid-step selection heuristics (areAllTickLabelsUnique / willLabelsFitInTickSpaceHint) seeing the exact same full set as before, so tick density on existing charts is unchanged.

What this issue tracks

Do the structurally cleaner version when there's time to validate it more broadly:

  1. Preferred: make AxisTickLabels clip tick labels to the plot-content band, the same way PlotSurface_AxesChart already clips gridlines and tick marks — so all three consumers of the tick list clip consistently, instead of the calculator emitting values one consumer must drop.

    or

  2. Generate ticks in-range from the start (fix getFirstPosition + the loop upper bound to emit exactly the nice multiples within [minValue, maxValue]), removing the overshoot entirely.

Why it wasn't done in #996
  • Changing generation inputs (option 2) alters what the termination heuristics see, which can change the selected grid step / tick density on charts that never had the bug — a wide regression surface with no pixel-baseline tests to catch it.
  • Option 1 needs the plot-content band (margin / tickSpace) plumbed into AxisTickLabels, which doesn't currently have it.
Bonus wart to fix along the way

With the current pruning approach, the user's tick-label formatting function is still called on the out-of-range values during generation (label computed, then discarded). A generate-in-range approach would avoid invoking the formatter on values outside the data band — relevant if a formatter has side effects or throws on unexpected inputs.

Acceptance
  • No regression to #634 (add-only; RegressionIssue634Test still passes).
  • Tick density/spacing unchanged across the demo gallery (needs a render pass to confirm).
  • Formatting function is not invoked for values outside [minValue, maxValue].
Lenguaje dominante
Java
Estrellas
1.6k
Forks
400
Merge medio
3 d 1 h
PR fusionados (30 d)
5

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de knowm/XChart

Todos los issues de knowm/XChart

Issues similares

Más issues de Java

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.