187849414 Autodetect Boundary Type #1613

tealefristoe · 2024-11-12T22:31:03Z

PT Stories:
https://www.pivotaltracker.com/story/show/187849414
https://www.pivotaltracker.com/story/show/188491735
https://www.pivotaltracker.com/story/show/188491712

This PR implements related behavior requested in three different stories.

Boundary attributes are now automatically detected.
- Just like in v2, attributes with one of several special names will be treated as a boundary attribute.
- Unlike in v2, an attribute will be considered of type boundary if its values all "look" like boundaries.
  - Bill started this one. I cleaned up the functions that he wrote a bit, but I suspect the way boundary data is recognized could be improved. Without having that many examples to work with or rules to follow, though, I just kept what he had.
Boundary values are rendered as thumbnail images in the table.
The lookupBoundary function has been implemented.
- This can most easily be seen in the Roller Coasters example document.
- Currently, we don't have a general solution for how to host boundary documents. Until we have that figured out, I added the state boundary document to the Codap. This means two things:
  - The document is included whenever Codap is loaded, not just when it is needed.
  - Only the state data is currently available, not any other boundary data.

Other changes:

basicCanonicalNameToDependency was moved from formula-dependency-utils.ts to name-mapping-utils.ts to avoid an import cycle. This was a very straightforward change to make. I'm not sure if there was some organizational reason for this function to be in one file rather than the other.

* Add geojson-utils.ts with `isBoundaryValue`

…tribute name to determine type.

codecov · 2024-11-12T22:32:33Z

Codecov Report

Attention: Patch coverage is 97.93814% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.63%. Comparing base (6f3b40e) to head (92adb98).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...3/src/models/formula/functions/lookup-functions.ts	97.29%	1 Missing ⚠️
v3/src/models/formula/utils/name-mapping-utils.ts	90.90%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1613       +/-   ##
===========================================
+ Coverage   67.88%   85.63%   +17.75%     
===========================================
  Files         600      602        +2     
  Lines       30405    30495       +90     
  Branches     8351     7809      -542     
===========================================
+ Hits        20639    26113     +5474     
+ Misses       9611     4227     -5384     
  Partials      155      155

Flag	Coverage Δ
cypress	`75.14% <91.76%> (+31.15%)`	⬆️
jest	`52.70% <91.75%> (+0.11%)`	⬆️

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

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

cypress · 2024-11-12T22:33:44Z

codap-v3 Run #5248

Run Properties: Passed #5248 • 92adb98571: Merge main.

Project	`codap-v3`
Branch Review	`187849414-autodetect-boundary-type-v3`
Run status	`Passed #5248`
Run duration	`06m 05s`
Commit	`92adb98571: Merge main.`
Committer	`Teale Fristoe`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`0`
Pending	`35`
Skipped	`0`
Passing	`221`
View all changes introduced in this branch ↗︎

kswenson

👍 Looks good -- suggestions are mainly to rearrange some of the dependencies.

kswenson · 2024-11-16T03:39:44Z

v3/src/models/formula/functions/lookup-functions.test.ts

+    expect(() => evaluate(`lookupBoundary("US_state_boundaries", "Alaska")`)).toThrow()
+    expect(() => evaluate(`lookupBoundary(Mammal, "Alaska")`)).toThrow()
+
+    // Second argument can't be a non-existant symbol


Suggested change

// Second argument can't be a non-existant symbol

// Second argument can't be a non-existent symbol

kswenson · 2024-11-16T03:57:55Z

v3/src/boundaries/boundaries.ts

I'm not quite sure what to do with this file, but maybe combine it with boundary-utils.ts (nee geojson-utils.ts) for now? Leave the src/boundaries folder for the GeoJSON files for the nonce, but it will go away once the boundary files are being loaded remotely, at which point maybe the rest of the relationships will become more clear as well.

kswenson · 2024-11-16T03:59:53Z

v3/src/models/data/attribute.ts

@@ -26,10 +26,12 @@
 */

 import { Instance, SnapshotIn, types } from "mobx-state-tree"
+import { kPolygonNames } from "../../components/map/map-types"


Attribute shouldn't be importing from the map component. I'd suggest moving kPolygonNames to utilities/boundary-utils.ts (nee geojson-utils.ts).

kswenson · 2024-11-16T04:00:17Z

v3/src/utilities/geojson-utils.test.ts

Rename to boundary-utils.test.ts.

kswenson · 2024-11-16T04:00:35Z

v3/src/utilities/geojson-utils.ts

Rename to boundary-utils.ts.

bfinzer and others added 5 commits October 30, 2024 08:28

[#187849414] Feature: Auto-detect boundary data type

091344c

* Add geojson-utils.ts with `isBoundaryValue`

Merge branch 'main' into 187849414-autodetect-boundary-type-v3

ac5b102

Bandaid bug in usePixiPointerDown. Clean up isBoundaryValue. Check at…

a9935ff

…tribute name to determine type.

Use same condition for strings and objects.

0433600

Display boundaries in table cells.

bd60b7d

tealefristoe marked this pull request as draft November 12, 2024 22:31

tealefristoe temporarily deployed to branches November 12, 2024 22:32 — with GitHub Actions Inactive

tealefristoe added 4 commits November 12, 2024 14:45

Merge main.

d72ec0f

Look up boundary data using formula function.

1f8a0e5

Save correct value from boundary document.

ee1cb8f

Merge branch 'main' into 187849414-autodetect-boundary-type-v3

c3319d9

tealefristoe added the v3 CODAP v3 label Nov 14, 2024

tealefristoe temporarily deployed to branches November 14, 2024 04:07 — with GitHub Actions Inactive

tealefristoe added 3 commits November 13, 2024 20:56

Add geojson-utils jest test.

7829656

Add jest test for lookupBoundary.

b7a77b2

Add cypress test to detect boundary thumbnails in table cells.

be0e7c4

tealefristoe added the run regression label Nov 14, 2024

tealefristoe temporarily deployed to branches November 14, 2024 18:31 — with GitHub Actions Inactive

Remove outdated file.

1d3f94e

tealefristoe temporarily deployed to branches November 14, 2024 19:23 — with GitHub Actions Inactive

Better match v2 lookupBoundary behavior and expand jest tests.

79a63b2

tealefristoe temporarily deployed to branches November 15, 2024 04:44 — with GitHub Actions Inactive

tealefristoe marked this pull request as ready for review November 15, 2024 13:28

tealefristoe requested a review from kswenson November 15, 2024 13:28

Merge main.

92adb98

tealefristoe deployed to branches November 16, 2024 03:55 — with GitHub Actions View deployment

kswenson requested changes Nov 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

187849414 Autodetect Boundary Type #1613

187849414 Autodetect Boundary Type #1613

tealefristoe commented Nov 12, 2024 •

edited

Loading

codecov bot commented Nov 12, 2024 •

edited

Loading

cypress bot commented Nov 12, 2024 •

edited

Loading

kswenson left a comment

kswenson Nov 16, 2024

kswenson Nov 16, 2024

kswenson Nov 16, 2024

kswenson Nov 16, 2024

kswenson Nov 16, 2024

	// Second argument can't be a non-existant symbol
	// Second argument can't be a non-existent symbol

187849414 Autodetect Boundary Type #1613

Are you sure you want to change the base?

187849414 Autodetect Boundary Type #1613

Conversation

tealefristoe commented Nov 12, 2024 • edited Loading

codecov bot commented Nov 12, 2024 • edited Loading

Codecov Report

cypress bot commented Nov 12, 2024 • edited Loading

codap-v3 Run #5248

kswenson left a comment

Choose a reason for hiding this comment

kswenson Nov 16, 2024

Choose a reason for hiding this comment

kswenson Nov 16, 2024

Choose a reason for hiding this comment

kswenson Nov 16, 2024

Choose a reason for hiding this comment

kswenson Nov 16, 2024

Choose a reason for hiding this comment

kswenson Nov 16, 2024

Choose a reason for hiding this comment

tealefristoe commented Nov 12, 2024 •

edited

Loading

codecov bot commented Nov 12, 2024 •

edited

Loading

cypress bot commented Nov 12, 2024 •

edited

Loading