Skip to content

Evitar raise del read_for_view cuando y_range="auto" está activado y no hay datos - #42

Merged
ecarreras merged 2 commits into
mainfrom
FIX_y_range_no_final_data
Oct 1, 2026
Merged

ecarreras merged 2 commits into
mainfrom
FIX_y_range_no_final_data

Conversation

@fcapallera

@fcapallera fcapallera commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Objetivos

  • Evitar un raise del read_for_view cuando y_range="auto" está activado y no hay datos.

Funcionamiento anterior

image
ValueError: The values array cannot be empty

Funcionamiento nuevo

image

Tests

  • mamba — 149 examples
  • Regression test verified to fail with ValueError when the final_data guard is removed.

@fcapallera fcapallera self-assigned this Oct 1, 2026
@fcapallera fcapallera added bug Something isn't working patch Create a Patch version labels Oct 1, 2026
@ecarreras
ecarreras requested a review from giscebot October 1, 2026 13:04

@giscebot giscebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

El canvi de producció és coherent: amb final_data buit manté yAxisOpts.mode = "auto", omet uns límits que no es poden calcular i evita el ValueError de get_min_max([]).

Cal afegir un test de regressió per al cas que motiva la PR. Ara mateix spec/graph/graph_spec.py només comprova y_range="auto" amb dades, de manera que eliminar accidentalment el and final_data continuaria deixant la suite verda. El test hauria de processar una gràfica lineal amb y_range="auto" i values=[], comprovar que no llança, que data == [] i que yAxisOpts == {"mode": "auto"}.

He revisat el diff complet, el contracte documentat de yAxisOpts, la implementació de get_min_max i el check build (verd).

@fcapallera

Copy link
Copy Markdown
Contributor Author

@giscebot podries afegir un test de regressió per comprovar el funcionament de la PR? Gràcies

Co-authored-by: fcapallera <36483261+fcapallera@users.noreply.github.com>
@giscebot

giscebot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Afegit el test de regressió al commit 722d19b. Cobreix y_range="auto" amb values=[] i comprova data == [] i yAxisOpts == {"mode": "auto"}. També he verificat que el test falla amb el ValueError original si es retira el guard de final_data; amb el canvi de la PR passen els 149 exemples de mamba.

@ecarreras
ecarreras requested a review from giscebot October 1, 2026 13:34
@ecarreras
ecarreras enabled auto-merge October 1, 2026 13:37

@giscebot giscebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

El bloqueig del review anterior queda resolt. El nou test reprodueix exactament el cas y_range="auto" amb values=[], valida tant data == [] com l’absència de límits inventats (yAxisOpts == {"mode": "auto"}), i queda acoblat al guard de producció: si es retira, torna a aparèixer el ValueError esperat.

He revisat el diff complet i la interacció amb get_min_max; el canvi conserva el comportament existent quan hi ha dades i només evita calcular mínim/màxim quan final_data és buit. El check build passa amb els 149 exemples. No he pogut repetir la suite localment perquè aquest entorn no té mamba ni les dependències (lxml) instal·lades, però no és un problema atribuïble al PR.

@ecarreras
ecarreras merged commit 6c85e27 into main Oct 1, 2026
1 check passed
@ecarreras
ecarreras deleted the FIX_y_range_no_final_data branch October 1, 2026 14:04
@giscebot

giscebot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Revisió tancada: el bloqueig del test de regressió va quedar resolt abans del merge. El commit de merge 6c85e27 inclou tant el guard sobre final_data com el test per y_range="auto" sense dades; el check build era verd (149 exemples). No queda cap acció pendent d’aquesta revisió.

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

Labels

bug Something isn't working patch Create a Patch version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants