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

"Extent" property for Regression does not modify the regression extent and nullifies the regression. #9230

Closed
chknfr1ed opened this issue Jan 19, 2024 · 5 comments · Fixed by #9312
Labels

Comments

@chknfr1ed
Copy link

I initially posted this in Vega and was suggested to move to here.

When I try to add the "extent" property as listed in the vega-lite documentation, the regression no longer exists and instead the data becomes a line chart.

This is the example pulled from the regression example in the vega-lite documentation:

image

However, when I add in the extent property to the example, this is the result:

image

@chknfr1ed
Copy link
Author

Someone else is also having this issue from SO: https://stackoverflow.com/questions/77845962/regression-line-issue-in-vegalite-charts

@jonmmease
Copy link
Contributor

When adding this to Vega-Lite:

"transform": [
    {"regression": "IMDB Rating", "on": "Rotten Tomatoes Rating", "extent": [0, 100]}
],

The generated Vega ends up with this transform:

    {
      "name": "data_1",
      "source": "source_0",
      "transform": [{"type": "extent", "field": [0, 100]}]
    },

This is nonsensical, as [0, 100] is not a field, and the extent transform doesn't bind the result to a signal anyway, so even if the "field" were valid, it wouldn't do anything.

@vega/maintainers Does anyone have context on how this is supposed to work in terms of the generated Vega? It seems to me like this needs to be a configuration option in the Vega transform, and it's not clear to me how this ever worked.

@thomascamminady
Copy link

FWIW, I filed the issue on altair's end here: vega/altair#3347

I was certain that this used to work and it seems that with altair=5.0.0 this is working as expected, but with altair=5.1.0 this does not work properly.

Looking at the release notes for 5.1.0 (https://github.com/altair-viz/altair/releases/tag/v5.1.0) this corresponds to the vega-lite update from version 5.8.0 to version 5.14.1.

Comparing both vega-lite versions can done here: v5.8.0...v5.14.1

I think I'm unable to pinpoint this further, but maybe this insight helps someone else :)

@player1537
Copy link

player1537 commented Apr 15, 2024

I can confirm that when I test with:

<script src="https://cdn.jsdelivr.net/npm/vega@5.25.0/build/vega.js"></script>
<script src="https://cdn.jsdelivr.net/npm/vega-lite@5.9.3/build/vega-lite.js"></script>
<script src="https://cdn.jsdelivr.net/npm/vega-embed@6.22.2/build/vega-embed.js"></script>

and vary the vega-lite version, then v5.9.3 works, and v5.10.0 does not work. (They all "work", but the latter has the bug this thread is about)

This brings the relevant changeset down to: v5.9.3...v5.10.0

Edit: After digging a little, the only relevant change is "feat: add extent transform (#8940)"

Some of the tests look an awful lot like the bug happening here.

{
    ...
      "transform": [{"extent": "b", "param": "b_extent" }],
    ...
}

->

{
    ...
      "transform": [{"type": "extent", "field": "b", "signal": "b_extent"}]
    ...
}

I think this might be a simple naming collision. The code for dealing with the extent transform is getting applied to the regression's extent property.

@lsh
Copy link
Member

lsh commented Apr 16, 2024

I see the issue. Will put up a fix ASAP.

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 a pull request may close this issue.

5 participants