tooltipID() helper for testing polygon hole by tdhock · Pull Request #328 · animint/animint2

tdhock · 2026-05-22T18:41:47Z

hi @Nishita-shah1 here is a stronger test for polygon holes #326 that fails on current master.

Refactor polygon rendering to use d3.nest for subgroup aesthetics and GeoJSON with evenodd fill rule.

tdhock · 2026-05-22T19:21:36Z

using the code in #326, all tests pass for me locally:

@Nishita-shah1 can you please add your code in this branch, and close #326 ?

More generally, for future PRs, can you please start by pushing a strong test like this that fails? And then push the fix after that?

tdhock · 2026-05-22T19:23:45Z

+mousePressed <- function(selector=NULL, ...)mouseEvent(selector, "mousePressed", ...)
+mouseReleased <- function(selector=NULL, ...)mouseEvent(selector, "mouseReleased", ...)
+clickIDpos <- function(id){


these are not actually used after all, because mouseMoved() is sufficient for this test. but I would like to keep them, because they may be useful for other tests in the future.

tdhock · 2026-05-23T11:55:16Z

I pushed an even stronger and simpler test: we look for a tooltip at all grid points, and there only should be a tooltip if num=1 (not 0) where the polygon should be drawn.

I get this failure locally:

> tests_run(filter="hole")
[1] "Polygon holes via subgroup aesthetic"
Saving _problems/test-renderer-polygon-holes-77.R
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1 ]

══ Failed tests ══════════════════════════════════════════════════════════════════════════════════════
── Failure (test-renderer-polygon-holes.R:77:3): polygon tooltip only shown for point with num=1 ─────
Expected `opacity` to equal `expected`.
Differences:
17/147 mismatches (average diff: 0.7)
[17] 0.7 - 0 == 0.7
[18] 0.7 - 0 == 0.7
[19] 0.7 - 0 == 0.7
[24] 0.7 - 0 == 0.7
[26] 0.7 - 0 == 0.7
[31] 0.7 - 0 == 0.7
[32] 0.7 - 0 == 0.7
[33] 0.7 - 0 == 0.7
[66] 0.7 - 0 == 0.7
...
Backtrace:
    ▆
 1. ├─computed.dt[, expect_equal(opacity, expected)] at test-renderer-polygon-holes.R:77:3
 2. └─data.table:::`[.data.table`(...) at test-renderer-polygon-holes.R:77:3
 3.   └─base::eval(jsub, SDenv, parent.frame())
 4.     └─base::eval(jsub, SDenv, parent.frame())
 5.       └─testthat::expect_equal(opacity, expected)

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 1 ]

tdhock · 2026-05-26T13:47:33Z

why does Files changed show binary *r files?

Nishita-shah1 · 2026-05-26T13:52:27Z

Hi Toby, Actually it's not the final commit , I was porting everything, I think there's some issue, I have to fix it. I will surely update when it will be corrected.

…

On Tue, 26 May 2026, 19:17 Toby Dylan Hocking, ***@***.***> wrote: *tdhock* left a comment (animint/animint2#328) <#328 (comment)> why does Files changed show binary *r files? — Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=BCA4SQNNOXVR3LHQYKNUX6344WOAZA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJUGQ3DINBRG4YKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4544644170>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BCA4SQOZYEB3MMWNICK6V3D44WOAZAVCNFSM6AAAAACZJUMIT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKNBUGY2DIMJXGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

tdhock · 2026-05-26T14:01:28Z

you should just copy files from the other branch, then git add, git commit, git push

Nishita-shah1 · 2026-05-26T14:14:01Z

Ohkie, I'll do this way.

…

On Tue, 26 May 2026, 19:31 Toby Dylan Hocking, ***@***.***> wrote: *tdhock* left a comment (animint/animint2#328) <#328 (comment)> you should just copy files from the other branch, then git add, git commit, git push — Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=BCA4SQKBBLAEF7EEK3MNZT344WPU7A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTINJUGQ3TOOJRGI42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4544779129>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BCA4SQLAKRCRIB65FV4WBUL44WPU7AVCNFSM6AAAAACZJUMIT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DKNBUG43TSMJSHE> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

codecov · 2026-05-27T18:54:09Z

Codecov Report

❌ Patch coverage is 31.42857% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.88%. Comparing base (426295c) to head (3bed39d).

Files with missing lines	Patch %	Lines
inst/htmljs/animint.js	31.42%	24 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
- Coverage   73.03%   72.88%   -0.16%     
==========================================
  Files         164      164              
  Lines        8840     8868      +28     
==========================================
+ Hits         6456     6463       +7     
- Misses       2384     2405      +21

Flag	Coverage Δ
javascript	`80.70% <31.42%> (-0.55%)`	⬇️
r	`69.14% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

tdhock · 2026-05-28T13:23:32Z

now the Files changed tab contains lots of irrelevant changes. Can you please fix?

Nishita-shah1 · 2026-06-27T05:05:33Z

Hi @tdhock ,
I am so sorry for misunderstanding from my side. After last comment I did keep the code style as you mentioned and used d3.geopath .
I will update this line return "M" + r.map(function(p){ return p[0] + "," + p[1]; }).join("L") + "Z";

other than this line is there any other changes to do too?
So sorry for delays, it will not happen again.

Nishita-shah1 · 2026-06-27T06:58:01Z

Hi @tdhock
I have updated the code, kept same as https://github.com/animint/animint2/pull/326/files,
removed "M" + r.map(function(p){ return p[0] + "," + p[1]; }).join("L") + "Z"; this code and use d3.geo.path() only. It is passing all the test cases. Please do let me know of any other changes.

Extremely sorry for the delay. It will not happen again.

tdhock

thanks!

tdhock · 2026-06-28T02:40:00Z

can you please now increase version in DESCRIPTION and add NEWS item?

Nishita-shah1 · 2026-06-28T03:16:19Z

Hi @tdhock , I have increased version in DESCRIPTION and added NEWS item.

tdhock

also please add your name as ctb in DESCRIPTION?

Co-authored-by: Toby Dylan Hocking <tdhock5@gmail.com>

tdhock · 2026-06-30T11:29:36Z

+              }
+              return ring;
+            });
+


please undo addition of empty lines

in JS functions

tdhock · 2026-06-30T11:31:43Z

+    person("Nishita", "Shah",
+     role="ctb",
+     comment="Animint2 GSoC 2026: polygon hole rendering via subgroup aesthetic in geom_polygon() (issue #252, PR#328)"))
+


please undo addition of empty line in DESCRIPTION

Co-authored-by: Toby Dylan Hocking <tdhock5@gmail.com>

Nishita-shah1 · 2026-07-05T07:37:22Z

Hi @tdhock
If everything is fine in this PR, could you please merge it .

tdhock · 2026-07-29T20:06:00Z

please fix conflicts and test failures?

Nishita-shah1 · 2026-07-31T07:52:53Z

Hi @tdhock i have fixed conflicts and test failures. I have ignored atime and codecov currently, rest all tests are passing.

nishita-shah1 and others added 6 commits April 4, 2026 19:57

fix issue #252, support subgroup aesthetic in geom_polygon for holes

3d5740b

Merge branch 'master' into fix/252-polygon-holes-subgroup

7c84313

Refactor polygon rendering for subgroup aesthetics

664157a

Refactor polygon rendering to use d3.nest for subgroup aesthetics and GeoJSON with evenodd fill rule.

tooltipID() helper for testing polygon hole

f976092

3 polygon test

90d9390

more uniform tests

e5fc26e

This comment was marked as outdated.

Sign in to view

tdhock commented May 22, 2026

View reviewed changes

mouseMoved in helper-functions

c26b817

tdhock mentioned this pull request May 22, 2026

simplify tooltip tests #329

Open

tdhock added 2 commits May 23, 2026 07:42

stronger and simpler test

7e8c5f9

remove simple test

c2ca989

tdhock mentioned this pull request May 23, 2026

Fix/252 polygon holes subgroup #326

Open

Nishita-shah1 mentioned this pull request May 24, 2026

Nishita's GSoC 2026 updates #322

Open

nishita-shah1 added 5 commits May 24, 2026 23:54

port polygon holes JS fix into test-polygon-hole branch, fixes #252

927b160

porting files from 326 to 328

25c47ba

Force-apply polygon subgroup rendering logic

9691ab7

add polygon subgroup fix from PR 326

919632a

force copy geom-polygon, geom-, animint.js from PR 326

a6bd40f

Nishita-shah1 added 3 commits May 27, 2026 23:10

correcting /inst/htmljs/animint

a928f77

porting geom-r file from pr 326

418796a

porting geom-polygon from pr 326 to pr 328

feb8794

correcting return way

7813cd2

kept code same pr 326

8796912

Nishita-shah1 force-pushed the test-polygon-hole branch 2 times, most recently from 4e08883 to 8796912 Compare June 27, 2026 10:21

tdhock commented Jun 28, 2026

View reviewed changes

tdhock marked this pull request as ready for review June 28, 2026 02:39

increaded version in desc and add news items

866d54f

tdhock commented Jun 29, 2026

View reviewed changes

Comment thread NEWS.md

Nishita-shah1 and others added 3 commits June 30, 2026 10:04

Update NEWS.md

d1c94d5

Co-authored-by: Toby Dylan Hocking <tdhock5@gmail.com>

add name as ctb

256cd06

news update

544ba9c

tdhock commented Jun 30, 2026

View reviewed changes

Comment thread DESCRIPTION Outdated

tdhock commented Jun 30, 2026

View reviewed changes

Nishita-shah1 and others added 4 commits July 1, 2026 10:01

Update DESCRIPTION

1e97878

Co-authored-by: Toby Dylan Hocking <tdhock5@gmail.com>

thanking myself in NEWS and removing extra lines

677e9a4

removing extra lines

bd1f4f1

Fix comment formatting in geom- function

bf384d0

Merge branch 'master' into test-polygon-hole

7f78949

nishita-shah1 added 2 commits July 31, 2026 03:17

Merge master into test-polygon-hole and fix DESCRIPTION Authors@R comma

ca95c2c

Merge remote test-polygon-hole

2ef5242

Nishita-shah1 force-pushed the test-polygon-hole branch from 964eafd to 3bed39d Compare July 31, 2026 07:39

Remove blank line from DESCRIPTION

3bed39d

Uh oh!

Conversation

tdhock commented May 22, 2026

Uh oh!

This comment was marked as outdated.

tdhock commented May 22, 2026

Uh oh!

tdhock May 22, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdhock commented May 26, 2026

Uh oh!

Nishita-shah1 commented May 26, 2026 via email

Uh oh!

tdhock commented May 26, 2026

Uh oh!

Nishita-shah1 commented May 26, 2026 via email

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

tdhock commented May 28, 2026

Uh oh!

Nishita-shah1 commented Jun 27, 2026

Uh oh!

Nishita-shah1 commented Jun 27, 2026

Uh oh!

tdhock left a comment

Choose a reason for hiding this comment

Uh oh!

tdhock commented Jun 28, 2026

Uh oh!

Nishita-shah1 commented Jun 28, 2026

Uh oh!

tdhock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tdhock Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tdhock Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Nishita-shah1 commented Jul 5, 2026

Uh oh!

tdhock commented Jul 29, 2026

Uh oh!

Nishita-shah1 commented Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tdhock commented May 23, 2026 •

edited

Loading

codecov Bot commented May 27, 2026 •

edited

Loading