docs+fix: ecarts doc/code resolus, read_rules parle a l'editeur de YAML#55
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR resolves doc↔code drift around compare_datasets_from_yaml() outputs and improves the ergonomics of hand-edited YAML rules by making read_rules() failures and typos more actionable, while updating README/vignette guidance accordingly.
Changes:
- Documentation alignment: README quick start now shows the zero-config path first (explicit
key), removes stale dev outputs, and the vignette documents extract diagnostic limitations + lazy NaN caveat. read_rules()andwrite_rules_template()now reject unsupported rules versions with clearer messaging;read_rules()also warns on unknown top-level/defaults fields.- Tests added to cover unsupported version errors, unknown-field warnings, and template version rejection.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/datadiff.Rmd | Documents local-only extract diagnostics and DuckDB-only NaN semantics for lazy booleans. |
| tests/testthat/test-main.R | Adds regression tests for actionable read_rules() errors/warnings and template version rejection. |
| README.Rmd | Reorders Quick Start to zero-config first; removes stale “Dev part” embedded outputs. |
| README.md | Knitted README reflecting the updated README.Rmd content and table formatting. |
| R/compare_datasets_from_yaml.R | Improves version validation in read_rules() and rejects unsupported write_rules_template() versions. |
| NEWS.md | Announces the documentation alignment + YAML friction improvements (issue #32). |
| man/compare_datasets_from_yaml.Rd | Regenerated docs reflecting updated @return items (all_passed + clarified agent). |
Files not reviewed (1)
- man/compare_datasets_from_yaml.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+136
to
+142
| if (is.null(r$version) || | ||
| !identical(suppressWarnings(as.numeric(r$version)), 1)) { | ||
| stop(sprintf( | ||
| "'%s' declares rules version %s; this version of {datadiff} supports version 1 only.", | ||
| path, r$version %||% "<missing>" | ||
| ), call. = FALSE) | ||
| } |
Member
Author
There was a problem hiding this comment.
Valides tous les deux (TDD rouge d'abord), appliques : la version non scalaire recoit un resume imprimable (classe + longueur) dans l'erreur prevue, et les 4 sections doivent etre des mappings YAML - refus explicite en amont au lieu du crash lointain sur $.
Comment on lines
+145
to
+149
| known_top <- c("version", "defaults", "by_type", "by_name", "row_validation") | ||
| unknown_top <- setdiff(names(r), known_top) | ||
| known_defaults <- c("na_equal", "ignore_columns", "keys", "key", "label") | ||
| unknown_defaults <- setdiff(names(r$defaults %||% list()), known_defaults) | ||
| unknown <- c(unknown_top, unknown_defaults) |
…ML (issue #32) - @return complet (all_passed en tete, agent decrit fidelement : configure mais non interroge) - README : Quick Start zero-config d'abord, cle explicite partout ; la section Dev n'embarque plus de sorties check/covr perissables (figees a la 0.4.2 depuis mars) - vignette : restriction locale des diagnostics __absdiff/__thresh dans les extraits ; caveat NaN DuckDB-only du chemin lazy (releve au triage de la PR #41) - read_rules : erreur de version explicite (fichier + version declaree + version supportee, fini le stopifnot brut) ; warning sur les champs inconnus racine/defaults (typos by_nmae, no_equal) en nommant les champs connus - la friction principale d'un YAML edite a la main - write_rules_template refuse version != 1 en amont (il ecrivait des templates que read_rules refusait ensuite)
VincentGuyader
force-pushed
the
fix/issue-32-doc-drift
branch
from
July 3, 2026 10:37
bd185ae to
38be24f
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 #32
Doc alignee sur le code
@returndecompare_datasets_from_yaml()liste les 8 elements dontall_passed(le premier, celui que tout consommateur lit) et decritagentfidelement : configure, NON interroge (reponsel'est)compare_datasets_from_yaml(ref, cand, key = "id")sans YAML), puis le flux template edite ;keyexplicite partoutcheck/covrperissables (figees a la 0.4.2 depuis mars) - remplacees par les commandes a lancer__absdiff/__threshdocumentee ; caveat NaN DuckDB-only du chemin lazy (l'overclaim « any DBI backend » releve au triage de la PR fix: le SQL temple reproduit la semantique NaN/Inf du kernel R #41)read_rules : friction d'un YAML edite a la main
stopifnotbrut « r$version == 1 is not TRUE »)by_nmae:,no_equal:) : warning nommant les champs fautifs et les champs connus au lieu de l'ignorance silencieusewrite_rules_template()refuseversion != 1en amont (il ecrivait des templates queread_rules()refusait ensuite)Tests (TDD red-first)
Erreur de version actionnable (rouge : le stopifnot brut), warnings de typos (rouge : silence), rejet de version du template (rouge : ecriture heureuse). Aucun YAML existant de la suite ne declenche les nouveaux warnings (verifie par la review interne : les 5 consommateurs de defaults correspondent exactement a la liste connue).
Suite complete : 1023 PASS / 0 FAIL. Check --as-cran 0/0.
Review interne
pr-reviewer : APPROVE, nit applique (suppressWarnings sur la coercition de version non numerique).