ThinkR-open / ThinkR-open/datadiff

[API] setup_pointblank_agent : paramètre cols_reference mort, vecteur de référence embarqué dans chaque step, exemple roxygen trompeur

Open
#25 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
R
Stars
6
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Contexte

setup_pointblank_agent() (R/pointblank_setup.R) est exportée mais présente plusieurs défauts d'API et de coût :

  1. Paramètre mort : cols_reference (2e position, documenté) n'apparaît nulle part dans le corps — vérifié par grep (seules occurrences : signature + doc). L'appelant calcule et passe cette valeur pour rien ; la variable locale cols_candidate de l'appelant est morte en cascade.
  2. Mémoire : sur le chemin local, chaque step d'égalité fait col_vals_equal(columns = c, value = cmp[[paste0(c, ref_suffix)]]) — le vecteur de référence entier est embarqué dans l'agent pour chaque colonne (O(n_rows) par step, sérialisé avec le rapport), et repose sur un value non scalaire hors du contrat documenté de pointblank.
  3. Exemple roxygen trompeur : l'exemple passe tol_cols = "b" alors que cmp n'a pas de colonne b__ok — l'agent produit échouerait à l'interrogation. Il masque le vrai contrat (booléens __ok précalculés par le pipeline).
  4. Export discutable : 15 paramètres dont 5 obligatoires sans défaut, couplage fort aux conventions internes (__ok, __eq, __missing_col_) ; en interne elle n'est plus appelée que sur le chemin d'échec avec add_col_exists_steps = FALSE. Par ailleurs get_col_names(cmp) est recalculé à chaque itération de la boucle des colonnes d'égalité.
  5. Les colonnes d'égalité défaillantes (fail$eq) sont passées dans le paramètre nommé common_cols — la sémantique du nom ment.

Reprex

library(datadiff)
# 1. paramètre mort : la valeur passée n'a aucun effet
cmp <- data.frame(a = 1:3, a__reference = 1:3)
r1 <- setup_pointblank_agent(cmp, cols_reference = c("a"),        common_cols = "a",
                             tol_cols = character(0), NULL, "__reference", .1, .1, "x", TRUE)
r2 <- setup_pointblank_agent(cmp, cols_reference = c("INEXISTANT"), common_cols = "a",
                             tol_cols = character(0), NULL, "__reference", .1, .1, "x", TRUE)
# aucune différence de comportement

Critères de succès

  • cols_reference retiré (dépréciation propre puisque exporté : warning une version, puis suppression) ; cols_candidate mort supprimé chez l'appelant.
  • Steps d'égalité locaux basés sur un booléen précalculé (cf. issue perf sur la mémoïsation __eq) au lieu d'embarquer le vecteur de référence.
  • Exemple roxygen exécutable et représentatif (avec __ok présent).
  • Décision documentée sur l'export : @keywords internal ou maintien assumé avec doc du contrat ; paramètre renommé ou documenté pour common_cols/fail$eq.
  • get_col_names(cmp) hissé hors boucle.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with R/pointblank_setup.R and trace its callers, especially where cols_candidate is computed and where fail$eq is passed as common_cols; inspect the local equality steps and the get_col_names(cmp) loop. Done means the dead parameter and caller variable are removed cleanly, equality steps use the precalculated boolean data, the roxygen example includes __ok, the export and naming decisions are documented, and get_col_names(cmp) is computed once outside the loop.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
backend-api-design, documentation, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.