Skip to content

feat(cli): complete analyze summary (span + speeds) and harden input - #12

Merged
felipebridge merged 5 commits into
felipebridge:mainfrom
jeremarche:feature/analyze-complete-summary
Sep 21, 2026
Merged

feat(cli): complete analyze summary (span + speeds) and harden input#12
felipebridge merged 5 commits into
felipebridge:mainfrom
jeremarche:feature/analyze-complete-summary

Conversation

@jeremarche

@jeremarche jeremarche commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

What does this change
Completes and hardens the analyze subcommand.

analyze advertises "recompute a summary from a previously exported tracks.csv",
but it only printed track counts. The export also carries per-track first/last
timestamps and speed estimates, so the summary now additionally reports the
tracked-activity span (earliest first_timestamp to latest last_timestamp) and
the average/fastest estimated vehicle speed — recomputed from the CSV alone, no
model re-run. The congestion level is deliberately not recomputed: it depends on
per-frame vehicle densities that only the run pass observes and that tracks.csv
does not store (noted in a code comment so the omission doesn't look accidental).

While there: pointing analyze at a CSV that lacks the expected columns (or at a
file that isn't a CSV at all) crashed with a raw KeyError / ParserError
traceback from inside pandas indexing. The expected columns are now validated up
front and the read is wrapped, so both cases log an actionable message and exit 1.
The read also uses utf-8-sig so exports round-tripped through Excel (BOM) parse.

Finally, the README never mentioned analyze (or that a CLI table exists at all),
so the subcommand was undiscoverable without reading the source; added a short CLI
section.

Related issue
Closes # (none opened — happy to file one if you'd rather track this there)

How was this tested

  • ruff check src tests dashboard scripts
  • pytest -q (86 passed: 80 pre-existing + 6 new)
  • Manually verified (describe below)
    Ran analyze on a generated tracks.csv: counts, Activity span,
    Average vehicle speed and Fastest vehicle print as expected; on a
    two-column CSV it now exits 1 with
    ... is missing expected column(s): class_name, first_timestamp, ...
    instead of a traceback.

Checklist

  • Tests added/updated for the change
    tests/test_cli_analyze.py: happy path (counts/span/speeds), no-speed
    calibration (n/a), missing columns, header-only export, non-CSV bytes,
    missing file.
  • Docs updated if behavior changed
    README CLI table + analyze help text now mention the recomputed stats.

Notes for the review, and ideas I'd be glad to take on as follow-up PRs if useful:

  • dashboard/data_loader.py and analyze both re-read tracks.csv; if the export
    schema grows, a shared "load + validate export" helper could keep them in sync.
  • CI only exercises Python 3.11 while pyproject.toml promises >=3.11; a
    3.11/3.12/3.13 matrix would catch drift early.
  • The Dockerfile / docker-compose.yml setup isn't mentioned in README or
    CONTRIBUTING, which is a shame for people who don't want a local torch install.

jere added 4 commits September 21, 2026 15:42
@jere
feat(cli): report activity span and speed stats in analyze
381b798 
The analyze subcommand advertises recomputing a summary from an exported
tracks.csv, but only printed track counts. The export also carries per-track
first/last timestamps and speed estimates, so the summary can additionally
report the tracked-activity span and the average/fastest vehicle speed of the
run without re-running the model.

The congestion level is deliberately not recomputed: it depends on per-frame
vehicle densities that only the run pass observes and that tracks.csv does not
store.
@jere
fix(cli): fail gracefully when analyze gets a non-tracks.csv input
161ee58 
Pointing `analyze` at a CSV that lacks the expected columns (or at a file
that is not a CSV at all) crashed with a raw KeyError / ParserError traceback
from inside pandas indexing. Validate the expected columns up front and wrap
the read, so both cases exit 1 with an actionable log message instead.

Also read with utf-8-sig so exports round-tripped through Excel (which adds a
BOM) still parse.
@jere
test(cli): cover analyze outcomes and bad-input handling
7b29686 
Locks in the new span/speed reporting and the graceful exit codes for a CSV
missing expected columns, a header-only export, a non-CSV file and a missing
file, so future changes to the export schema or the subcommand surface
regressions immediately.
@jere
docs: document the CLI subcommands, including analyze
adffc64 
The README showed run and dashboard in the quickstart but never mentioned
analyze, so the subcommand was effectively undiscoverable without reading the
source. Add a short CLI table with an analyze example.
@jeremarche

jeremarche commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Aclaración de transparencia: este PR lo preparé yo (@jeremarche) con ayuda sustancial de un
asistente de IA, que me acompañó a analizar el código base y a redactar los cambios y los
tests. Las decisiones fueron mías (qué aportar, cómo validarlo —ruff y 86/86 pytest en local
más verificación manual del antes/después— y seguir el flujo fork → rama → PR del
CONTRIBUTING), y asumo la responsabilidad completa por todo lo que hay en este diff.

Lo digo de entrada en lugar de dejar que aparezca más adelante: si alguna parte del cambio
no se sostiene en la revisión, prefiero que me lo marquen y corregirla aprendiendo en el
camino. Con gusto recorro cualquier fragmento del diff con el nivel de detalle que haga
falta.

@felipebridge

felipebridge commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Gracias por el PR, buen trabajo — probé el diff completo a mano (tests, ruff, y algunos casos borde) y los números que ponés en la descripción son correctos.

Encontré un bug antes de aprobar: except (pd.errors.ParserError, UnicodeDecodeError) no cubre pandas.errors.EmptyDataError, que es justo lo que tira pd.read_csv con un archivo de 0 bytes (no "solo header" — vacío de verdad, ej. una exportación que falló a mitad de camino o un touch accidental). Lo reproduje:

$ python -m traffic_intelligence analyze --input empty.csv
...
pandas.errors.EmptyDataError: No columns to parse from file

Es el mismo traceback crudo que este PR dice estar eliminando. EmptyDataError es hermana de ParserError (ambas heredan de ValueError, no una de la otra), por eso test_analyze_rejects_header_only_csv no lo agarra — ese CSV tiene header, no está realmente vacío.

¿Podés sumar pd.errors.EmptyDataError al tuple del except (o pasar a except ValueError, ya que las tres cuelgan de ahí) y agregar un test con un archivo de 0 bytes?

El resto lo apruebo tal cual: la validación de columnas, el cálculo de activity span y velocidad, el utf-8-sig para BOM de Excel, y el README/help actualizados. Con ese fix, lo mergeo.

@jeremarche

jeremarche commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Hola Felipe,

¡Gracias por la review y por el repro tan preciso! Tenías razón en todo:
EmptyDataError es hermana de ParserError (ambas cuelgan de ValueError), por eso
ni el except ni el test de header-only lo agarraban — ese CSV sí parsea columnas,
solo que cero filas; el de 0 bytes ni eso.

Apliqué tu opción A: sumé pd.errors.EmptyDataError al tuple del except (elegí
el tuple explícito en vez de except ValueError para que los modos de falla
cubiertos queden documentados en el propio catch) y agregué
test_analyze_rejects_zero_byte_file con un archivo de 0 bytes reales.

Va como un commit nuevo encima de los que ya revisaste (sin reescribir historia,
para que tu review siga leyendo igual): ruff limpio y 87/87 pytest en local, y
reproduje tu caso a mano antes y después del fix — ahora sale el mensaje claro y
exit 1 en vez del traceback.

Gracias de nuevo por el nivel de detalle: se aprende un montón de una review así.

Jeremías

@jeremarche
fix(cli): handle EmptyDataError from zero-byte files in analyze
73b4f89 
A 0-byte input (an export that died mid-write, an accidental `touch`)
makes pd.read_csv raise EmptyDataError, which the except tuple missed:
EmptyDataError is a sibling of ParserError (both subclass ValueError),
not a subclass of it, so the raw traceback this subcommand promises to
eliminate still escaped for that case. The header-only test could not
catch it either: a header-only CSV parses zero rows but does parse
columns, so it never raises EmptyDataError.

Add EmptyDataError to the tuple and cover the 0-byte case with its own
test. Kept the explicit tuple rather than broadening to `except ValueError`
so the caught failure modes stay documented at the catch site.

Spotted by @felipebridge in PR felipebridge#12 review; repro and diagnosis his.
@felipebridge
felipebridge merged commit dbb9433 into felipebridge:main Sep 21, 2026
@jeremarche

jeremarche commented Sep 21, 2026 via email

Copy link
Copy Markdown
Contributor Author

@felipebridge felipebridge mentioned this pull request Sep 21, 2026
Open
@jeremarche

jeremarche commented Sep 24, 2026 via email

Copy link
Copy Markdown
Contributor Author

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jeremarche @felipebridge @jere