chore: menage packaging, imports et scories#56
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR is a maintenance-focused cleanup addressing issue #34: it removes orphan packaging artifacts, trims unused imports/dependencies, and applies small performance/clarity refactors in hot-path code without changing user-visible behavior (per PR description and green test suite).
Changes:
- Packaging/deps cleanup: remove unused imports (
dplyr::arrange/across,stats::setNames) and dropstatsfromDESCRIPTION; delete orphaninst/templates/rules_template.yaml. - Performance/memory tweaks in comparison internals (tolerance kernels, coverage counting, and lazy preprocessing query construction).
- Minor readability cleanups (loop variable renames to avoid shadowing
base::c, clearer numeric literal defaults, reduced duplication for Arrow class detection).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| R/utils.R | Deduplicates Arrow class checks via is_arrow() reused by is_non_local(). |
| R/tolerance.R | Hoists repeated computations out of kernels/loops; documents integer overflow caveat; avoids c shadowing. |
| R/preprocessing.R | Refactors lazy normalization to accumulate expressions and apply a single mutate() (dbplyr plan size reduction). |
| R/pointblank_setup.R | Renames loop variables to avoid shadowing base::c. |
| R/duplicate_keys.R | Truncates displayed key groups before formatting examples (less work on large key sets). |
| R/coverage.R | Counts tolerance failures without allocating intermediate logical vectors; avoids c shadowing. |
| R/compare_datasets_from_yaml.R | Replaces stats::setNames usage with base construction; updates defaults/readability; avoids c shadowing. |
| NEWS.md | Documents the packaging + internal cleanup outcomes and deliberate non-changes. |
| NAMESPACE | Removes unused imports (across, arrange, setNames). |
| inst/templates/rules_template.yaml | Deletes orphan template file. |
| DESCRIPTION | Drops stats from Imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+24
to
+27
| #' @note Integer columns are compared through the same double arithmetic: | ||
| #' \code{cand - ref} on integer vectors can overflow to NA with a warning | ||
| #' near \code{.Machine$integer.max}; cast to double upstream if your keys | ||
| #' or values approach that range. |
Member
Author
There was a problem hiding this comment.
Valide : la soustraction est bien entiere quand les deux colonnes le sont (le double n'intervient qu'apres, pour thresh), et « keys » n'avait rien a faire la. Note reecrite : overflow de la DIFFERENCE au-dela de integer.max -> NA + warning, NA compte comme echec, caster en double en amont.
- .Rbuildignore ^dev$ (le RFC et les benchs ne partent plus dans le tarball) ; inst/templates/rules_template.yaml orphelin supprime - imports : dplyr arrange/across retires, stats::setNames remplace par une construction base ; numeric_abs = 1e-9 lisible - is_non_local reutilise is_arrow (liste de classes Arrow definie une fois) ; abs(ref_vals) et la constante IEEE hisses hors des kernels et boucles ; tol_col_counts compte sans allocations intermediaires ; format_key_examples tronque avant de formater - preprocess_dataframe lazy : UNE seule mutate() composee pour toutes les colonnes normalisees (anti-pattern O(colonnes) dbplyr) - variables de boucle c renommees (masquaient base::c) dans pointblank_setup, coverage, tolerance, compare - note d'overflow integer documentee sur le kernel - ecartes explicitement (wontfix motives dans NEWS) : rendu gt en session non interactive (print = rendu), garde DBI transitive via duckdb
VincentGuyader
force-pushed
the
chore/issue-34-cleanup
branch
from
July 3, 2026 10:41
75d489c to
b928347
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #34
Packaging
inst/templates/rules_template.yamlorphelin supprime (reference nulle part, verifiesystem.file)dplyr::arrange/acrossretires du NAMESPACE (arrange inutilise, across toujours qualifie),stats::setNamesremplace par une construction base (stats sort de DESCRIPTION - c'etait la note du check) ;^dev$etait deja arrive par la PR docs: RFC surface d'API 0.5 (design a discuter, rien d'implemente) #54Scories de code (equivalences verifiees par la review interne)
numeric_abs = 1e-9lisible (au lieu du litteral a 9 zeros)is_non_local()reutiliseis_arrow()(liste de classes Arrow definie une fois)abs(ref_vals)et la constante IEEE hisses hors des kernels et boucles (bit-identique, garde d'equivalence verte)tol_col_counts():length(ok) - sum(ok, na.rm = TRUE)sans allocations intermediaires (equivalence algebrique + empirique TRUE/FALSE/NA)format_key_examples()tronque a 3 groupes AVANT de formaterpreprocess_dataframe()lazy : UNEmutate()composee pour toutes les colonnes normalisees (l'anti-pattern O(colonnes) dbplyr elimine partout ailleurs en 0.4.8)crenommeescol_nm(masquaientbase::c) dans 4 fichiersEcarte sciemment (motive dans NEWS)
Suite complete : 1023 PASS / 0 FAIL. Check --as-cran 0 erreur / 0 warning (la note stats disparait avec cette PR).
Review interne
pr-reviewer : APPROVE (equivalences setNames/structure, comptage NA, head-avant-format verifiees empiriquement), nit d'indentation applique.