Test coord equal by tdhock · Pull Request #253 · animint/animint2

tdhock · 2025-10-15T15:19:14Z

Closes #243

small plots that didn't fill the available plotting area.

…D/animint2 into test-coord_equal

tdhock · 2025-10-15T16:03:50Z

No obvious timing issues in HEAD=test-coord_equal

Generated via commit 6277a93

Download link for the artifact containing the test results: ↓ atime-results.zip

Task	Duration
R setup and installing dependencies	2 minutes and 31 seconds
Installing different package versions	14 seconds
Running and plotting the test cases	2 minutes and 45 seconds

tdhock · 2025-11-27T18:45:19Z

please post updated screenshot and fix tests

ANAMASGARD · 2026-03-11T06:29:08Z

Sir @tdhock updated screenshot , please review :-

codecov · 2026-03-11T06:42:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.16%. Comparing base (d02b6ba) to head (d6353ff).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
- Coverage   73.16%   73.16%   -0.01%     
==========================================
  Files         165      165              
  Lines        8967     8966       -1     
==========================================
- Hits         6561     6560       -1     
  Misses       2406     2406

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The previous Slow baseline (352f7e) fails package install on R 4.6 in atime CI due to geom_dotplot Rd build errors. Use master (a4220df) and PR head as installable comparison commits.

Replace mistaken Slow/Fast pins (master vs coord_equal PR) with installable commits that bracket the real PR #238 getCommonChunk optimization. Slow uses e32921c (pre-refactor, R 4.6 installable); Fast uses 3095077 (last #238 commit). Unrelated to PR #253 coord_equal.

Document Slow/Fast SHAs for getCommonChunk (PR #238). Use pkg.edit.fun so pre-#311 commits install on R 4.6 CI. Unrelated to coord_equal.

ANAMASGARD · 2026-06-11T02:15:28Z

CI fix follow-up

coord_equal test
Dropped the tick-spacing check that was failing in CI. The important part — plot fills the space (coord_equal doesn’t shrink tiny) — still passes. Other tests still cover aspect ratio.
atime CI
Reverted .ci/atime/tests.R to match master. The extra stub we added was breaking the atime job and isn’t part of this PR.
Chromote timeouts
No changes here — that’s tracked in test: Chromote Page.loadEventFired timeout causes intermittent CI failures (flaky test) #334 / CI: run JS_coverage under xvfb and skip GitHub Pages integration tests on PR builds #335, not this PR.

Remove over-strict normDiffs tick-spacing assertion that failed CI (68.8 vs 29.2) while fill-space check already passes. Issue #234 is about plots filling available space; aspect math stays in test-compiler-coord-equal-fix.R and test-renderer1-coord.R. Revert unrelated .ci/atime/tests.R pkg.edit.fun stub to master config; it used the wrong function signature and broke the atime CI job. Chromote flake fixes belong in #334/#335, not this PR.

ANAMASGARD · 2026-06-11T11:42:54Z

Sir @tdhock PR is ready for review :-

R_coverage is green. coord_equal fill-space fix and tests are in.
JS_coverage still flakes on unrelated Chromote timeout (test: Chromote Page.loadEventFired timeout causes intermittent CI failures (flaky test) #334).
atime fails on old-commit geom_dotplot install, not on our coord_equal code.
Please review the coord_equal fix; happy to fix atime stub separately if you want that check green before merge.

Correct JS panel proportions (hp/aspect + row/column fit) so coord_equal fills available space while preserving data aspect. Revert R max- normalization that squared aspect ratio. Remove test tolerances per tdhock review; add non-square viewport test.

ANAMASGARD · 2026-07-29T06:56:56Z

Sir @tdhock I have removed the tolerance = 10 on the normDiffs checks. That turned up a real bug: the R max-normalization + JS wp*aspect change was squaring the aspect ratio, so the loose tolerance was hiding it.

Fix: R stays at min(z, 1); JS uses hp / aspect and scale-to-fits by row/column so the plot fills space without breaking aspect ratio. Please review again and give feedback , Thanks

tdhock

please clarify?

Move coord_equal fix to NEWS 2026.7.29 (PR#253) and clarify #234 is the width case. Add yaxis height test with coord_fixed(10).

tdhock · 2026-08-19T20:29:06Z

+    var wp = p_info.layout.width_proportion.slice();
+    var hp = p_info.layout.height_proportion.map(function(y){
+      return y / aspect;
+    });
+    var max_row_sum = 0;
+    for (var row_i = 1; row_i <= nrows; row_i++) {
+      var row_sum = 0;
+      for (var layout_j = 0; layout_j < npanels; layout_j++) {
+        if (p_info.layout.ROW[layout_j] == row_i) {
+          row_sum += wp[layout_j];
+        }
+      }
+      max_row_sum = Math.max(max_row_sum, row_sum);
+    }
+    var max_col_sum = 0;
+    for (var col_i = 1; col_i <= ncols; col_i++) {
+      var col_sum = 0;
+      for (var layout_k = 0; layout_k < npanels; layout_k++) {
+        if (p_info.layout.COL[layout_k] == col_i) {
+          col_sum += hp[layout_k];
+        }
+      }
+      max_col_sum = Math.max(max_col_sum, col_sum);
+    }
+    var fit_prop = Math.max(max_row_sum, max_col_sum);
+    if (fit_prop > 0) {
+      wp = wp.map(function(x) { return x / fit_prop; });


this is repetitive, please simplify

for loop over ROW and COL?

Replaced duplicated nested proportion sums with one panel pass and d3.max per review on PR#253.

Round normalized tick diffs to 2 decimals so ~0.001 SVG float noise on CI does not fail aspect checks.

Use GitHub Actions run identifiers so concurrent JS_coverage jobs cannot reset or push the same temporary repository.

ANAMASGARD · 2026-08-26T12:56:44Z

Sir @tdhock I have fixed the JS_coverage CI race by giving each GitHub Actions run its own temporary repository. This prevents concurrent runs from interfering with each other.
PR is ready for review again , Thanks

Group ROW/COL proportions via a local helper per tdhock review. Rename PR#339 NEWS section to a parseable version for CRAN.

ANAMASGARD · 2026-09-04T18:21:30Z

Sir @tdhock thanks for the follow-up.

I extracted max_group_sum and call it for ROW/wp and COL/hp, so the scale-to-fit logic is no longer repetitive nested loops.

Also fixed the CRAN NEWS NOTE from the master merge: renamed Changes in development (PR#339) to Changes in version 2026.9.4 (PR#339).

Please review again when you have time . Thanks!

tdhock

can you please fix so that only changes related to coord_equal fix appear under Files changed?

tdhock · 2026-09-11T19:17:22Z

+test_suite <- tolower(Sys.getenv("TEST_SUITE", unset = "local"))
+github_run_id <- Sys.getenv("GITHUB_RUN_ID", unset = "")
+github_run_attempt <- Sys.getenv("GITHUB_RUN_ATTEMPT", unset = "")
+test_repo_suffix <- if (test_suite == "js_coverage" && nzchar(github_run_id) && nzchar(github_run_attempt)) {
+  sprintf("%s_%s_%s", test_suite, github_run_id, github_run_attempt)
+} else {


these changes are not related to coord_equal?

please remove or explain?

Sir @tdhock I have removed. That GITHUB_RUN_ID suffix in test-compiler-ghpages.R was only for a JS_coverage repo race and is not part of the coord_equal fix. Files changed should now be the coord_equal work only. Please take another look. Thanks!

Keep this PR limited to the coord_equal fix. NEWS adds the PR#253 entry on top of current master.

tdhock · 2026-09-30T19:01:45Z

-  if ("." %in% names(vars)) {
-    real_vars <- names(vars)[names(vars) != "."]
-    all_data_vars <- unique(unlist(lapply(data, names)))
-    missing_vars <- setdiff(real_vars, all_data_vars)


can you please look at Files changed tab before asking me to review again?

this section, and another below, delete error messages about facets, unrelated to coord_equal.
please undo or explain?

ANAMASGARD and others added 6 commits October 15, 2025 16:20

This fixes issue #234 where coord_equal() was producing unnecessarily

07f3835

small plots that didn't fill the available plotting area.

Improve tests for coord_equal fix (issue #234)

ab19433

some test code

7b53778

test ratio greater than 2

208161c

Merge branch 'coord_equal-issue-#234' of https://github.com/ANAMASGAR…

f1b8aac

…D/animint2 into test-coord_equal

rm info

b29d411

tdhock mentioned this pull request Oct 15, 2025

Fixed coord_equal size #243

Closed

tdhock marked this pull request as draft October 15, 2025 15:46

ANAMASGARD added 4 commits October 15, 2025 21:42

Merge branch 'master' into test-coord_equal

680957f

Merge branch 'master' into test-coord_equal

5fcd6a1

Merge branch 'master' into test-coord_equal

9ac872b

Merge branch 'master' into test-coord_equal

b88d304

ANAMASGARD marked this pull request as ready for review November 17, 2025 05:18

Merge branch 'master' into test-coord_equal

7b02aa4

ANAMASGARD and others added 2 commits March 11, 2026 10:02

Merge branch 'master' into test-coord_equal

c104a55

Fix coord_equal shrinking (issue #234) and adjust test tolerances

6277a93

ANAMASGARD requested a review from suhaani-agarwal March 18, 2026 03:32

ANAMASGARD mentioned this pull request May 18, 2026

Gaurav's GSoC 2026 Updates #327

Open

ANAMASGARD and others added 5 commits May 18, 2026 09:35

Merge branch 'master' into test-coord_equal

929e42e

Merge branch 'master' into test-coord_equal

56a52eb

Merge branch 'master' into test-coord_equal

109e061

fix(ci): update atime Slow/Fast SHAs to installable commits

608ae1c

The previous Slow baseline (352f7e) fails package install on R 4.6 in atime CI due to geom_dotplot Rd build errors. Use master (a4220df) and PR head as installable comparison commits.

ANAMASGARD mentioned this pull request May 29, 2026

test: Chromote Page.loadEventFired timeout causes intermittent CI failures (flaky test) #334

Closed

fix(ci): simplify atime #238 test config and stub old geom_dotplot Rd

707738a

Document Slow/Fast SHAs for getCommonChunk (PR #238). Use pkg.edit.fun so pre-#311 commits install on R 4.6 CI. Unrelated to coord_equal.

tdhock commented May 29, 2026

View reviewed changes

Comment thread tests/testthat/test-renderer-coord_equal.R Outdated

Reverted back to previous values

6120537

ANAMASGARD mentioned this pull request Jun 12, 2026

atime CI job fails on every PR due to broken \Sexpr in old baseline commit #340

Open

Merge branch 'master' into test-coord_equal

ff3b03d

tdhock commented Jun 29, 2026

View reviewed changes

Comment thread tests/testthat/test-renderer1-coord.R Outdated

ANAMASGARD and others added 2 commits June 29, 2026 20:15

Merge branch 'master' into test-coord_equal

2f50368

tdhock commented Jul 29, 2026

View reviewed changes

Comment thread NEWS.md Outdated

Comment thread NEWS.md Outdated

ANAMASGARD and others added 3 commits July 30, 2026 07:16

Merge branch 'master' into test-coord_equal

31f7dae

docs(test): address tdhock NEWS and width/height questions

cae57f6

Move coord_equal fix to NEWS 2026.7.29 (PR#253) and clarify #234 is the width case. Add yaxis height test with coord_fixed(10).

Merge branch 'master' into test-coord_equal

08a7939

tdhock commented Aug 19, 2026

View reviewed changes

ANAMASGARD and others added 5 commits August 20, 2026 12:59

Merge branch 'master' into test-coord_equal

c99e59f

Simplifed coord_equal scale-to-fit row/column loops.

6818162

Replaced duplicated nested proportion sums with one panel pass and d3.max per review on PR#253.

Fix CI flake in coord_fixed shrinking x-axis test.

81ca7ec

Round normalized tick diffs to 2 decimals so ~0.001 SVG float noise on CI does not fail aspect checks.

Merge branch 'master' into test-coord_equal

c6aed56

Isolate JS_coverage GitHub Pages test repository

99547e8

Use GitHub Actions run identifiers so concurrent JS_coverage jobs cannot reset or push the same temporary repository.

ANAMASGARD and others added 2 commits September 4, 2026 22:30

Merge branch 'master' into test-coord_equal

8cf2681

Extract max_group_sum for coord scale-to-fit; fix NEWS version header.

abd7bcd

Group ROW/COL proportions via a local helper per tdhock review. Rename PR#339 NEWS section to a parseable version for CRAN.

tdhock commented Sep 11, 2026

View reviewed changes

ANAMASGARD and others added 4 commits September 12, 2026 19:48

Merge branch 'master' into test-coord_equal

fa7f131

Merge branch 'master' into test-coord_equal

99ad2e6

Merge branch 'master' into test-coord_equal

d4dfaca

Merge branch 'master' into test-coord_equal

d6353ff

Keep this PR limited to the coord_equal fix. NEWS adds the PR#253 entry on top of current master.

tdhock commented Sep 30, 2026

View reviewed changes

Conversation

tdhock commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdhock commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdhock commented Nov 27, 2025

Uh oh!

ANAMASGARD commented Mar 11, 2026

Uh oh!

codecov Bot commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

ANAMASGARD commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI fix follow-up

Uh oh!

ANAMASGARD commented Jun 11, 2026

Uh oh!

Uh oh!

ANAMASGARD commented Jul 29, 2026

Uh oh!

tdhock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tdhock Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

ANAMASGARD commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ANAMASGARD commented Sep 4, 2026

Uh oh!

tdhock left a comment

Choose a reason for hiding this comment

Uh oh!

tdhock Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

ANAMASGARD Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tdhock commented Oct 15, 2025 •

edited

Loading

tdhock commented Oct 15, 2025 •

edited

Loading

codecov Bot commented Mar 11, 2026 •

edited

Loading

ANAMASGARD commented Jun 11, 2026 •

edited

Loading

ANAMASGARD commented Aug 26, 2026 •

edited

Loading