ThinkR-open / ThinkR-open/datadiff
[bug] Divergence de verdict local vs lazy sur NaN/Inf : le SQL templé ne reproduit pas la sémantique du kernel R
Nobody has claimed this yet.
- Dominant language
- R
- Stars
- 6
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
Contexte
Le kernel local compute_tolerance_col() (R/tolerance.R) a une sémantique complète NaN/Inf : Inf même signe des deux côtés → PASS, NaN/Inf unilatéral → FAIL, NaN bilatéral → suit na_equal. Le SQL templé add_bool_cols_sql() ne gère que NULL (CASE WHEN IS NULL). Or dans DuckDB NaN n'est pas NULL : NaN est ordonné supérieur à tout et NaN = NaN est vrai.
Conséquences (mêmes données, verdicts opposés selon le backend) :
| Cas | Kernel R (local) | SQL (lazy) |
|---|---|---|
ref = NaN, cand fini |
FAIL | ABS(c-r) et seuil = NaN → NaN <= NaN → faux PASS |
ref = ±Inf, cand fini |
FAIL | seuil = NaN ou Inf → faux PASS |
Inf même signe des deux côtés |
PASS | ABS(Inf-Inf) = NaN ≤ seuil fini → faux FAIL |
NaN des deux côtés, na_equal = FALSE |
FAIL | NaN = NaN → faux PASS |
Des Parquet contenant NaN/Inf lus nativement par DuckDB sont directement exposés. La doc de add_bool_cols_sql ne revendique que l'équivalence avec l'ancien case_when dplyr — vrai (même trou), donc divergence préexistante à 0.4.8, mais jamais signalée.
Angle mort structurel des tests : test-lazy-sql-bool.R utilise l'ancien chemin dplyr (add_tolerance_columns lazy) comme oracle — qui a le même trou. Il ne peut par construction détecter aucune divergence local↔lazy, et aucun cas NaN/Inf n'existe sur le chemin SQL.
Reprex
library(datadiff)
library(duckdb); library(dplyr)
ref <- data.frame(id = 1:4, x = c(NaN, Inf, Inf, NaN))
cand <- data.frame(id = 1:4, x = c(0, 1, Inf, NaN))
# Chemin local : lignes 1-2 FAIL, ligne 3 PASS, ligne 4 suit na_equal
res_local <- compare_datasets_from_yaml(ref, cand, key = "id")
res_local$coverage
# Chemin lazy DuckDB : verdicts inversés sur les 4 lignes
con <- dbConnect(duckdb())
duckdb_register(con, "ref", ref); duckdb_register(con, "cand", cand)
res_lazy <- compare_datasets_from_yaml(tbl(con, "ref"), tbl(con, "cand"), key = "id")
res_lazy$coverage
identical(res_local$all_passed, res_lazy$all_passed) # devrait être TRUE
Critères de succès
- Le CASE WHEN de
add_bool_cols_sql()reproduit la sémantique du kernel R viaisnan()/isinf()DuckDB (et équivalents SQLite testés), y compris le cap dufp_correctionnon fini. - Le reprex ci-dessus donne des verdicts identiques local vs DuckDB sur les 4 cas.
- Nouveau test d'équivalence local↔DuckDB (et SQLite) sur données NaN/Inf/NA, avec le kernel R comme oracle — pas l'ancien chemin dplyr.
- La doc d'
add_bool_cols_sql()et deadd_tolerance_columns()documente la sémantique NaN/Inf garantie sur chaque chemin.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with compute_tolerance_col() in R/tolerance.R and add_bool_cols_sql(), then inspect test-lazy-sql-bool.R to understand the existing oracle. Add local↔DuckDB and SQLite equivalence coverage for NaN, Inf, and NA, including fp_correction, using the R kernel as the oracle. Update the documentation for add_bool_cols_sql() and add_tolerance_columns() so the guaranteed NaN/Inf semantics are explicit.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r, sql, sqlite
- Domain
- databases, documentation, testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100