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

naming parameter views of layered repeated charts + interactivity #2849

Closed
mattijn opened this issue Jan 23, 2023 · 10 comments · Fixed by #3031
Closed

naming parameter views of layered repeated charts + interactivity #2849

mattijn opened this issue Jan 23, 2023 · 10 comments · Fixed by #3031

Comments

@mattijn
Copy link
Contributor

mattijn commented Jan 23, 2023

As discussed in this comment: vega/vega-lite#8348 (comment). It is not really clear how we implemented naming of view in top-level parameters for a layered repeater.

This was partly discussed before here: #2684 (comment), and potentially initially implemented in this commit: f547323 (this commit was not merged), but this PR #2702 was merged where the actual change is included.

Moreover, the behavior we implemented previously does work in the following situation, -example 1- (using altair from main branch on github):


Example 1

import altair as alt
from vega_datasets import data
iris = data.iris.url

alt.Chart(iris).mark_point().encode(
    alt.X(alt.repeat("column"), type='quantitative'),
    alt.Y(alt.repeat("row"), type='quantitative'),
    color='species:N'
).properties(
    width=160,
    height=130
).repeat(
    row=['petalLength', 'petalWidth'],
    column=['sepalLength', 'sepalWidth']
).interactive().to_dict()
{
  "$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json",
  "config": {
    "view": {
      "continuousHeight": 300,
      "continuousWidth": 300
    }
  },
  "params": [
    {
      "bind": "scales",
      "name": "param_20",
      "select": {
        "encodings": [
          "x",
          "y"
        ],
        "type": "interval"
      },
      "views": [
        "view_24child__row_petalLengthcolumn_sepalLength",
        "view_24child__row_petalLengthcolumn_sepalWidth",
        "view_24child__row_petalWidthcolumn_sepalLength",
        "view_24child__row_petalWidthcolumn_sepalWidth"
      ]
    }
  ],
  "repeat": {
    "column": [
      "sepalLength",
      "sepalWidth"
    ],
    "row": [
      "petalLength",
      "petalWidth"
    ]
  },
  "spec": {
    "data": {
      "url": "https://cdn.jsdelivr.net/npm/[email protected]/data/iris.json"
    },
    "encoding": {
      "color": {
        "field": "species",
        "type": "nominal"
      },
      "x": {
        "field": {
          "repeat": "column"
        },
        "type": "quantitative"
      },
      "y": {
        "field": {
          "repeat": "row"
        },
        "type": "quantitative"
      }
    },
    "height": 130,
    "mark": "point",
    "name": "view_24",
    "width": 160
  }
}

Specifically note this part within parameters:

"views": [
  "view_24child__row_petalLengthcolumn_sepalLength",
  "view_24child__row_petalLengthcolumn_sepalWidth",
  "view_24child__row_petalWidthcolumn_sepalLength",
  "view_24child__row_petalWidthcolumn_sepalWidth"
]

Example 2

But the following specification:

source = alt.UrlData(
    data.flights_2k.url,
    format={'parse': {'date': 'date'}}
)

brush = alt.selection(type='interval', encodings=['x'])

# Define the base chart, with the common parts of the
# background and highlights
base = alt.Chart(source).mark_bar().encode(
    x=alt.X(alt.repeat('column'), type='quantitative', bin=alt.Bin(maxbins=20)),
    y='count()'
).properties(
    width=160,
    height=130
)

# gray background with selection
background = base.encode(
    color=alt.value('#ddd')
).add_params(brush)

# blue highlights on the transformed data
highlight = base.transform_filter(brush)

# layer the two charts & repeat
chart = alt.layer(
    background,
    highlight,
    data=source
).transform_calculate(
    "time",
    "hours(datum.date)"
).repeat(column=["distance", "delay", "time"])

leads to:

{
  "$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json",
  "config": {
    "view": {
      "continuousHeight": 300,
      "continuousWidth": 300
    }
  },
  "params": [
    {
      "name": "param_26",
      "select": {
        "encodings": [
          "x"
        ],
        "type": "interval"
      },
      "views": [
        "view_29"
      ]
    }
  ],
  "repeat": {
    "column": [
      "distance",
      "delay",
      "time"
    ]
  },
  "spec": {
    "data": {
      "format": {
        "parse": {
          "date": "date"
        }
      },
      "url": "https://cdn.jsdelivr.net/npm/[email protected]/data/flights-2k.json"
    },
    "height": 130,
    "layer": [
      {
        "encoding": {
          "color": {
            "value": "#ddd"
          },
          "x": {
            "bin": {
              "maxbins": 20
            },
            "field": {
              "repeat": "column"
            },
            "type": "quantitative"
          },
          "y": {
            "aggregate": "count",
            "type": "quantitative"
          }
        },
        "mark": "bar",
        "name": "view_29"
      },
      {
        "encoding": {
          "x": {
            "bin": {
              "maxbins": 20
            },
            "field": {
              "repeat": "column"
            },
            "type": "quantitative"
          },
          "y": {
            "aggregate": "count",
            "type": "quantitative"
          }
        },
        "mark": "bar",
        "transform": [
          {
            "filter": {
              "param": "param_26"
            }
          }
        ]
      }
    ],
    "transform": [
      {
        "as": "time",
        "calculate": "hours(datum.date)"
      }
    ],
    "width": 160
  }
}

Specifically, our params looks as such:

"params": [
  {
    "name": "param_26",
    "select": {
      "encodings": [
        "x"
      ],
      "type": "interval"
    },
    "views": [
      "view_29"
    ]
  }
]

Where the vega-lite specification should be defined similar to this Open the Chart in the Vega Editor as is mentioned in this comment

Which contains a params definition as follows:

"params": [
  {
    "name": "brush",
    "select": {"type": "interval", "encodings": ["x"]},
    "views": [
      ["child__column_distance", "layer_0"],
      ["child__column_delay", "layer_0"],
      ["child__column_time", "layer_0"]
    ]
  }
],

As can be observed the views object is different.

We might have missed something here. Maybe this is because example 2 contains a layered repeater object, where example 1 does not contain a layered repeater.

@mattijn
Copy link
Contributor Author

mattijn commented Feb 5, 2023

This issue is halted by vega/vega-lite#8733 which will define the strategy that Altair should take.

@ChristopherDavisUCI
Copy link
Contributor

vega/vega-lite#8662 might also be worth keeping an eye on.

@mattijn
Copy link
Contributor Author

mattijn commented Apr 15, 2023

With VL5.7 released in editor I can see that it now behaves as follows:

"params": [
  {
    "name": "brush",
    "select": {"type": "interval", "encodings": ["x"]},
    "views": [
        "child__column_distance_layer_0",
        "child__column_delay_layer_0",
        "child__column_time_layer_0"
    ]
  }
],

So there is no list-nesting needed, now "child__column_distance_layer_0" vs previous ["child__column_distance", "layer_0"].

Open the Chart in the Vega Editor

@mattijn
Copy link
Contributor Author

mattijn commented Apr 16, 2023

@ChristopherDavisUCI, do you have time available to work on this in the coming week? Otherwise will try as well.

@ChristopherDavisUCI
Copy link
Contributor

Hi @mattijn, in terms of looking at the Altair code, I definitely have time this week, it's more getting everything updated to the current place that is the sticking point for me...

@mattijn could you briefly summarize the issue for me? Was this interactive crossfilter example working in a previous version (using our new parameter naming convention) and then it broke, or was it never working?

@mattijn
Copy link
Contributor Author

mattijn commented Apr 16, 2023

Yes will do! The issue is currently only about

  • layered repeat charts + interactivity

From the moment we introduced top-level defined parameters this has never worked (due not proper initiating the views object within params).

I think the following approach should be adopted, for the Interactive Crossfilter example:

  1. Defining a name in the first object of the layer within the spec:
  "spec": {
    "layer": [{
      "name": "view_1",
  1. Checking what is being repeated.
  "repeat": {"column": ["distance", "delay", "time"]}
  1. Construct the views property in the params:
  "params": [
    {
      "name": "param_1",
      "select": {"encodings": ["x"], "type": "interval"},
      "views": [
        "child__column_distance_view_1",
        "child__column_delay_view_1",
        "child__column_time_view_1"
      ]
    }
  ],

Where the view is defined as, child__<repeat-row>_<data-field><repeat-column>_<data-field>_<view-name>

See here a working specification how it should become: Open the Chart in the Vega Editor

@mattijn mattijn changed the title revisit naming of parameter views of layered repeated charts naming parameter views of layered repeated charts + interactivity Apr 17, 2023
@mattijn
Copy link
Contributor Author

mattijn commented Apr 21, 2023

@ChristopherDavisUCI, I just merged PR in the Altair main repo, so the main branch of Altair is on VL5.7.1. I hope this will make it easier to work on this issue.

Here a few steps that maybe helps getting up your local environment again after all the recent changes:

  1. Make sure the master branch of your published fork is synchronized with the master of altair main repo.
    You can click Sync fork on your github page: https://github.com/ChristopherDavisUCI/altair, and click Update branch
    sync_branch
    If this succeed, this button will grey out and the row states a message: This branch is up to date with altair-viz/altair:master.
    You also can use git from the terminal by setting up a remote, that you can fetch, merge and push from your forked master branch.

  2. Make sure your local master branch has the latest changes from step 1.
    If you have conflicts, be careful, but if the changes are not recent, its probably easiest to discard your local changes.
    Within your development environment (eg. conda); within the cloned altair folder of your fork, you can do

git checkout master
git pull
  1. Update development dependencies
    Make sure have the required dependencies in your development environment.
    This should at least add hatch as a new dependency and upgrades the packages altair_viewer and vl-convert-python.
    Make sure altair_viewer has access to vega-lite 5.7.1
    I also had to uninstall altair_saver to not pollute the tests-results.
python -m pip install -e .[dev]
python -m pip install vl-convert-python --upgrade --no-deps
>>> from altair_viewer import get_bundled_script
>>> get_bundled_script("vega-lite", '5.7.1')
pip uninstall altair_saver
  1. Correct Python interpreter within VSCode
    If you are running the test-suite within VSCode make sure you also have selected the python interpreter connected to the right environment with the installed dependencies of step 3.

  2. Then create a new branch from your local master, named eg, 'fix_gh2849`.

  3. Apply your changes
    As was mentioned in Update from Vega-Lite 5.6.1 to 5.7.1 #3022 (comment), you will need to activate the interactive_layered_crossfilter example in the tests again by removing the leading _ from the filename _interactive_layered_crossfilter.py in both the examples_arguments_syntax and examples_methods_syntax folder.

  4. Test, commit, push, publish and create a PR
    Apply and test your other changes, commit the messages to your branch, push the branch and create a PR from your published branch to the main altair repo. You can also test your changes locally from the altair root folder within your branch of step 5 using:

hatch run test

If you need help, just ping!

Edit:
I was not able to have hatch reinstall the altair_viewer package when running hatch run test, using hatch env remove test I could remove the environment before reinstalling it. See also pypa/hatch#594 (reply in thread) (and to less extent https://hatch.pypa.io/latest/config/hatch/#directories).

@ChristopherDavisUCI
Copy link
Contributor

Thanks @mattijn, I hope to find some time this weekend!

@ChristopherDavisUCI
Copy link
Contributor

ChristopherDavisUCI commented Apr 23, 2023

Those detailed instructions were very helpful @mattijn, thank you! I think my development environment is caught up. I will keep you updated... I need to refresh my memory on the logic of where the view naming is currently happening.

Update: My first guess is that at least part of the problem is due to these lines:
https://github.com/altair-viz/altair/blob/8a66cbec80119812d2ef09b6ffb802d7ea302dbc/altair/vegalite/v5/api.py#L2664-L2665
I don't remember exactly what issue I was trying to avoid when I included them. My first initial attempts (including adding an extra underscore like @binste pointed out in the linked Vega-Lite issue) have not worked to get this interactive crossfilter working.

@mattijn
Copy link
Contributor Author

mattijn commented Apr 23, 2023

That is a separate issue, #3024, namely a repeat chart with params.
Here it is a layered repeat chart with params.
Maybe it should happen in the Layer context?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants