Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.17] [ML] Data Frame Analytics: Fix field name escaping for Vega based scatterplot matrix. (#193386) #193841

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

walterra
Copy link
Contributor

Backport

This will backport the following commits from main to 7.17:

Questions ?

Please refer to the Backport tool documentation

…tterplot matrix. (elastic#193386)

Field names with `\n` would fail to render the DFA scatterplot matrix:

<img width="804" alt="image"
src="https://github.com/user-attachments/assets/26e356b8-236d-4255-b556-2ebc2e5db4fc">

This fixes the escaping and adds unit tests.

The fix isn't 100% ideal because there are cases when we may end up with
an additional backslash being rendered for labels of the scatterplot.
However, all other variations I tried caused rendering problems of the
charts and rendering would fail completely.

For example, just escaping `\n` without the general backslash escaping
causes the following Vega error: `Duplicate scale or projection name:
"child__row_my_numbercolumn_my_number_x"`

On the other hand escaping just the backslash without the additional
`\n` escaping causes an "expression parse error" in in Vega and the
chart wouldn't render.

Note this PR just focuses on escaping for the Vega spec for the
scatterplot matrix. There are still other places in the UI (data grid
headers, fields selector).

<img width="792" alt="image"
src="https://github.com/user-attachments/assets/35532741-7a13-4707-b8da-c72dcc8c935b">

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 194d630)
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 3.3MB 3.3MB +30.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @walterra

@walterra walterra merged commit 36547fa into elastic:7.17 Sep 24, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants