fix: contradictions NEWS/code resolues - %||% interne, label comparison, doc lang exacte#53
Conversation
There was a problem hiding this comment.
Pull request overview
This PR resolves multiple NEWS/documentation vs code contradictions introduced after the 0.4.4 release (issue #26), aligning runtime behavior and documentation around %||% export status, default template label prefix, and the vignette’s language-default description. It also documents previously-exported helper functions in the vignette rather than internalizing them.
Changes:
- Re-internalizes
%||%(removes export/docs) and updates the corresponding test expectation. - Changes the default
write_rules_template()label prefix to"comparison"and updates tests accordingly. - Updates the vignette to document the real default language/locale and adds a “utility functions” section for exported helpers.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/datadiff.Rmd | Documents helper functions and corrects the documented default language/locale behavior. |
| tests/testthat/test-utils.R | Updates tests to assert `% |
| tests/testthat/test-input-validation.R | Updates expectations for the default label prefix (comparaison → comparison). |
| R/utils.R | Makes `% |
| R/compare_datasets_from_yaml.R | Switches the default label prefix to "comparison" when label is NULL/empty. |
| NEWS.md | Adds a NEWS entry describing the resolved contradictions and documentation updates. |
| NAMESPACE | Removes `export("% |
| man/or_operator.Rd | Removes the generated Rd for the formerly-exported `% |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test_that("%||% is internal (not exported), per NEWS 0.4.4", { | ||
| # Exporting it masks rlang's and base R's (R >= 4.4) own %||% at load time | ||
| expect_false("%||%" %in% getNamespaceExports("datadiff")) |
There was a problem hiding this comment.
Faux positif de la meme classe que ceux refutes sur les PRs #43/#44 : tests/testthat.R utilise test_check("datadiff"), dont l'environnement de test est un clone de asNamespace("datadiff") - dans cet environnement, %||% resout vers l'implementation INTERNE de datadiff, pas vers rlang/base (verifie empiriquement sous le harnais installe, test_dir(load_package = "installed")). Le test exerce bien notre operateur.
| By default, pointblank reports are rendered in French (`lang = "fr"`, | ||
| `locale = "fr_FR"`; override globally with `options(datadiff.lang = "en", | ||
| datadiff.locale = "en_US")`). You can change | ||
| the language per call or globally for a session. |
There was a problem hiding this comment.
Valides (celui-ci et le tempfile) : span inline recompose sur une ligne, et les deux ecritures cwd des tests de label migres sur tempfile + on.exit - deux survivantes du sweep #31, merci.
| template_path <- "test_null_label.yaml" | ||
| write_rules_template(df, key = "id", label = NULL, path = template_path) | ||
| rules <- read_rules(template_path) | ||
| expect_match(rules$defaults$label, "comparaison") | ||
| expect_match(rules$defaults$label, "comparison") | ||
| unlink(template_path) |
…on, doc lang exacte (issue #26) Le commit Perf (#1) post-0.4.4 avait re-exporte %||% (masquant rlang::%||% et base::%||% R >= 4.4 au chargement), re-bascule le label par defaut en francais et laisse la vignette pretendre un defaut anglais - le tout sans entree NEWS, contredisant les annonces de la 0.4.4. - %||% re-internalise (conforme NEWS 0.4.4) ; le test qui verrouillait l'export accidentel verrouille desormais l'inverse - prefixe de label du template : comparison (annonce 0.4.4) - vignette : defaut de langue reel documente (fr / fr_FR + options) - normalize_text, validate_row_counts, setup_pointblank_agent documentes dans la section utilitaires de la vignette avec cas d'usage reels (ils n'apparaissaient nulle part)
69bc4f3 to
e20f848
Compare
Closes #26
Contexte
Le commit « Perf (#1) » post-0.4.4 avait re-exporte
%||%, re-bascule le label par defaut en francais et laisse la vignette pretendre un defaut anglais - le tout sans entree NEWS, contredisant les annonces de la release 0.4.4.Decisions (dans le sens annonce par la 0.4.4, ou doc alignee sur le reel)
%||%re-internalise : l'export masquaitrlang::%||%etbase::%||%(R >= 4.4) au chargement ; le test qui verrouillait l'export accidentel verrouille desormais l'inverse. Pas de rupture vs CRAN : la 0.4.4 publiee ne l'exportait deja pas.normalize_text,validate_row_counts,setup_pointblank_agent) sont documentes dans la vignette (section utilitaires, cas d'usage reels) plutot qu'internalises - le retrait serait cassant, propose au RFC 0.5.Suite complete : 1023 PASS / 0 FAIL sur la pile. Check --as-cran 0/0.
Review interne
pr-reviewer : REQUEST CHANGES applique (narration historique retiree du commentaire %||%, seul le rationale durable reste) + nit vignette (arguments nommes).