Skip to content

Write each weighted JSON item with rawweighting and value, keeping weight and rawWeight for this release - #139

Merged
jwrosewell merged 8 commits into
rcfrom
fix/json-value-forms
Nov 15, 2026
Merged

jwrosewell merged 8 commits into
rcfrom
fix/json-value-forms

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The JSON Builder writes a Property's value as the Element Data gives it (pipeline/fiftyone.pipeline.core/jsonbundler.js line 271 at 7c99e780), with only a list of Element Data rewritten (lines 242 to 246). So each item of a weighted value in the JSON sent to the browser has the members the Element Data uses, value, weight and rawWeight (ip-intelligence/fiftyone.ipintelligence.onpremise/swigHelpers.js line 59), where the specification writes rawweighting and value, in lower case. A cloud weighted value carries rawweighting as well since #124, and an on-premise one does not.

What the specification says

From pipeline-specification/features/weighted-values.md lines 45 to 58 at 706de1f0:

When serializing weighted values, they are represented like:

{
    "weightedpropertyname": [
        { "rawweighting": 32768, "value": "value 1" },
        { "rawweighting": 32767, "value": "value 2" }
    ]
}

The keys MUST be written in lower case, as the other keys of the JSON Builder Element are. A reader MUST accept them in lower case, which is how the remote server writes them.

What changed

The JSON Builder writes each item of a weighted value, being a list whose items each have a value and a numeric rawWeight or rawweighting, with rawweighting and value, taking rawweighting from rawWeight where an item has it. Each item keeps the weight and rawWeight it had beside them, deprecated, so client code that reads them still works in this release. Any other list is written item by item as #118 writes it, and any other value as it is. The Element Data is unchanged, so only the JSON sent to the browser changes, and nothing in types changes.

Before you upgrade

Nothing breaks. Client-side code that reads weight or rawWeight from a weighted value in the JSON, for example through fod.complete, should read rawweighting and value instead, because weight and rawWeight there are deprecated and a later release removes them.

Tests

The new pipeline/fiftyone.pipeline.core/tests/jsonValueForms.test.js builds a Pipeline with an element whose Property metadata and values are given, and reads the JSON the JSON Builder writes. It checks that a weighted string value given as the on-premise engines give one is written with rawweighting and value and keeps weight and rawWeight, that an item that already has rawweighting, as the cloud engine's do, is written the same way, and that a string and a list of strings are written as they are.

Red, run 43673171880 on 6c9edb3, these tests on rc 1ed293b, with only a throwaway workflow added that runs the core package's tests. On Ubuntu and on Windows 1 of 559 fails, being the test of a weighted value given as the on-premise engines give one, which rc writes with no rawweighting. The test of an item that already has rawweighting passes on rc, which writes such an item as it is.

Green, run 43673180942 on 79ad330, with only the same throwaway workflow added. All 537 pass on Ubuntu and on Windows, lint passes on the core package, and npm run tsc writes the committed types again with no difference but pipeline/fiftyone.pipeline.did/types/owid-js/v1.d.ts, which #127 decides.

Review then found two comments that still described the WKT change, which dddd7e9 corrects. Green on this head, run 43679273729 on dddd7e9, with only the same throwaway workflow added. All 537 pass on Ubuntu and on Windows, lint passes on the core package, and npm run tsc writes the committed types again with no difference but pipeline/fiftyone.pipeline.did/types/owid-js/v1.d.ts.

An earlier form of this change, which also wrote WKT values as objects and left out weight and rawWeight, ran red in run 43656112507 and green in run 43659878531. Review asked for both names to stay for a release and for the WKT form to wait, as for #124.

Full build

Run 43679502429 on tmp/pr-matrix-139-d, started by tools/pr-matrix.py without macOS. All 7 Setting 'PullRequestSha' lines give this head, dddd7e9, and Configure printed "Left out 2 macOS entries". Each of the six Build and Test jobs, Ubuntu, Ubuntu ARM and Windows on Node.js 22 and 24, passed 1,606 unit tests with 2 skipped and 119 integration tests with 4 skipped.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

  1. A WKT value is still written as the text the on-premise engine gives, where value-types.md lines 123 to 130 at 706de1f0 ask for an object with one member, value.
  2. weight and the camel case rawWeight stay on each weighted item in the JSON.

Both wait for a release that may change the form of a value, as #147 records, with the IP Intelligence cloud engine's own values in #138.

@jwrosewell jwrosewell changed the title Write WKT values and weighted values to the JSON in the specification's forms Write each weighted JSON item with rawweighting and value, keeping weight and rawWeight for this release Nov 15, 2026
@jwrosewell
jwrosewell marked this pull request as ready for review November 15, 2026 06:01
@jwrosewell
jwrosewell merged commit 085433b into rc Nov 15, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the fix/json-value-forms branch November 15, 2026 06:01
@jwrosewell jwrosewell mentioned this pull request Nov 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant