Skip to content

feat: add sort function to vega-functions (and vega-interpreter)#3973

Merged
domoritz merged 5 commits intomainfrom
cameron.yick/sort-functions-register-supplement
Sep 25, 2024
Merged

feat: add sort function to vega-functions (and vega-interpreter)#3973
domoritz merged 5 commits intomainfrom
cameron.yick/sort-functions-register-supplement

Conversation

@hydrosquall
Copy link
Member

@hydrosquall hydrosquall commented Sep 21, 2024 •
edited
Loading

Motivation

This PR tests a fix for #3962 , with the goal of adding a sort function to the default set of Vega functions in the expression language.

Changes

Testing

  • This worked locally when using yarn link vega
  • This is also working in the Cloudflare preview

Uses test spec JSON:

{
  "$schema": "https://vega.github.io/schema/vega/v5.json",
  "background": "white",
  "padding": 5,
  "width": 20,
  "style": "cell",
  "data": [
    {"name": "source_0", "values": [{"a": "A", "b": 28}]},
    {
      "name": "data_0",
      "source": "source_0",
      "transform": [
        {"type": "formula", "expr": "sort(['f', 'a', 't', 'b'])", "as": "test"}
      ]
    }
  ],
  "signals": [
    {"name": "y_step", "value": 20},
    {
      "name": "height",
      "update": "bandspace(domain('y').length, 1, 0.5) * y_step"
    }
  ],
  "marks": [
   
  ],
  "scales": [
    {
      "name": "y",
      "type": "point",
      "domain": {"data": "data_0", "field": "test", "sort": true},
      "range": {"step": {"signal": "y_step"}},
      "padding": 0.5
    }
  ],
  "axes": [
    {
      "scale": "y",
      "orient": "left",
      "grid": false,
      "title": "test",
      "zindex": 0
    }
  ]
}

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 21, 2024 •
edited
Loading

Deploying vega with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5e6190d
Status: ✅  Deploy successful!
Preview URL: https://8a4c23d3.vega-628.pages.dev
Branch Preview URL: https://cameron-yick-sort-functions.vega-628.pages.dev

View logs

@hydrosquall hydrosquall changed the base branch from main to sort-functions September 21, 2024 17:16
@hydrosquall hydrosquall mentioned this pull request Sep 21, 2024
Closed
@hydrosquall hydrosquall changed the title Cameron.yick/sort functions register supplement feat: add sort expressions to vega-expression (in addition to vega-interpreter) Sep 21, 2024
@hydrosquall hydrosquall changed the title feat: add sort expressions to vega-expression (in addition to vega-interpreter) feat: add sort expressions to vega-functions (in addition to vega-interpreter) Sep 21, 2024
@hydrosquall hydrosquall force-pushed the cameron.yick/sort-functions-register-supplement branch 3 times, most recently from 3daa1d7 to a105f40 Compare September 21, 2024 17:51
@hydrosquall hydrosquall force-pushed the cameron.yick/sort-functions-register-supplement branch from a105f40 to 5e6190d Compare September 21, 2024 17:53
@hydrosquall hydrosquall changed the title feat: add sort expressions to vega-functions (in addition to vega-interpreter) feat: add sort function to vega-functions (in addition to vega-interpreter) Sep 22, 2024
@hydrosquall hydrosquall changed the title feat: add sort function to vega-functions (in addition to vega-interpreter) feat: add sort function to vega-functions (in addition to vega-interpreter) Sep 22, 2024
@hydrosquall hydrosquall changed the base branch from sort-functions to main September 22, 2024 00:25
@domoritz domoritz changed the base branch from main to sort-functions September 23, 2024 11:49
@domoritz domoritz changed the base branch from sort-functions to main September 23, 2024 11:50
@hydrosquall hydrosquall self-assigned this Sep 23, 2024
@hydrosquall hydrosquall marked this pull request as ready for review September 23, 2024 16:51
@hydrosquall hydrosquall requested a review from a team as a code owner September 23, 2024 16:51
@hydrosquall hydrosquall changed the title feat: add sort function to vega-functions (in addition to vega-interpreter) feat: add sort function to vega-functions (and vega-interpreter) Sep 23, 2024
@joelostblom
Copy link
Contributor

joelostblom commented Sep 23, 2024

This is great, thank you @hydrosquall !

One thing that I noticed is that when there is a mix of numbers and strings, it seems like this function sorts them as subarrays: first it sorts all the string until the first number, then all the numbers, then all the string after the last number:

image

I would expect the output to be ["0", "1", "2", "3", "4", "a", "b"]. But to be honest, I don't think this is a show stopper; it would be rare that anyone passes in an array with mixed object types and it is easy to just pass in all strings instead.

@hydrosquall
Copy link
Member Author

hydrosquall commented Sep 24, 2024

Thanks for having a close look @joelostblom , it's cool to see how those subarrays are handled in Javascript sorting!

I'm not sure what to expect if we mix datatypes beyond the known sentinel values like null and undefined. The sorting would likely look strange if we mixed boolean and object ( {name : "hello"} ) values into the array too. I think the current behavior of not having sort coerce datatypes is preferred. If the user needed their 1 to be treated as strings they have the option of coercing them to strings before the sort, or vice versa (parsing string as int).

domoritz
domoritz approved these changes Sep 25, 2024
View reviewed changes
Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Looks great.

@domoritz domoritz enabled auto-merge (squash) September 25, 2024 01:50
@domoritz domoritz merged commit fb2e602 into main Sep 25, 2024
@domoritz domoritz deleted the cameron.yick/sort-functions-register-supplement branch September 25, 2024 01:52
@joelostblom
Copy link
Contributor

joelostblom commented Sep 25, 2024

Thank you both!

dangotbanned added a commit to vega/altair that referenced this pull request Sep 27, 2024
@dangotbanned
feat: Bump source to add alt.expr.sort
01e61e3 
See vega/vega#3973
@hydrosquall hydrosquall mentioned this pull request Jan 19, 2025
Open
@lsh lsh mentioned this pull request Jan 24, 2025
Merged
lsh added a commit that referenced this pull request Jan 24, 2025
@lsh
chore: v5.31.0 (#4003)
0e1eb8b 
changes since v5.30.0

**vega-utils**
* use `Object.hasOwn` instead of `Object.prototype.hasOwnProperty` (via
#3951). (Thanks @domoritz!)

**vega-parser**
* Add discrete legend type (via #3957). (Thanks @hydrosquall!)

**vega-functions**
* Add sort function to vega-functions (and vega-interpreter) (via
#3973). (Thanks @hydrosquall!)

**vega-selections**
* Add field predicate types to selectionTest (via #3675). (Thanks
@jonathanzong!)

**monorepo**
* Replace flights-5k.json external reference (via #3999). (Thanks
@dwootton!)

**docs**
* Update packed bubble example (via #3955). (Thanks @PBI-David!)
* Correct typo in production rules documentation (via #3958). (Thanks
@shanebruggeman!)
* Update README.md to fix broken link to current roadmap (via #3979).
(Thanks @cahogan & @joelostblom!)

---------

Signed-off-by: Lukas Hermann <1734032+lsh@users.noreply.github.com>
@dangotbanned dangotbanned mentioned this pull request Jan 28, 2025
Merged
1 task
@hydrosquall hydrosquall mentioned this pull request Feb 4, 2025
Merged
hydrosquall added a commit that referenced this pull request Feb 15, 2025
@hydrosquall
feat: Add base64 string encoder/decoder to vega-expression and `veg…
a3af49e 
…a-interpreter` (#4009)

## Motivation

- Allow users to modify SVG images that were Base64 encoded. This is
useful in Vega-Lite environments where remote access to images was
disabled, such as Deneb (PowerBI), so users are especially dependent on
inline data definitions.
- See #3875 

## Testing

- Try test URL sandbox showing that images can be encoded/decoded
[link](https://cameron-yick-feature-add-bas.vega-628.pages.dev/#/url/vega-lite/N4IgJAzgxgFgpgWwIYgFwhgF0wBwqgegIDc4BzJAOjIEtMYBXAI0poHsDp5kTykBaADZ04JAKyUAVhDYA7EABoQAEySYUqUMSSCGcCGgDaoDSACCikEzQAmABwBfBSbQgAQpeuoxYpy-QAwp5oACwAzA4Auk4gyABOANauTEhxlnCyUGzKNLJkaKAAngUgAGY0cILKrhlZynDVSpiFOHCusmwIuTogMQAeJeWV1eg09bKYdMVNLW3oHV2yPTFZgmxpmmUVVa4oM63tnd2CloLkGSOyDIKCDjGYcUiyEKXrCEagUDpQ12pzIGo2EwABRMTBsJDA1SYBgIShIACUCMsSAMo3Gk2avWcIC+gh+gj+yXBkOhsPhyKUqJqmWyDV60SAA)
- Make sure it works with AND without vega-interpreter mode:
vega/editor#1460
- I did not unit test this change yet as the function is a core web API.
However, if it would be useful to have at least 1 spec that uses both
atob and btoa, I can add one.

## Notes

- btoa / atob: check the status for node.js usage (
DefinitelyTyped/DefinitelyTyped#65494 ) since
the Node.js world prefers using `Buffer.from(` instead.
- Check why/if this is different from what we did for vega-functions in
#3973
- To get the scenegraph for my test, I visited
https://cameron-yick-feature-add-bas.vega-628.pages.dev/#/edited, used
and used `VEGA_DEBUG.view` to get a sample scenegraph.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@domoritz domoritz domoritz approved these changes

Assignees

@hydrosquall hydrosquall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hydrosquall @joelostblom @domoritz