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

Simplify syntax #2991

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Simplify syntax #2991

merged 1 commit into from
Mar 28, 2023

Conversation

joelostblom
Copy link
Contributor

As per the discussion in #2988 (reply in thread)

@joelostblom joelostblom requested a review from mattijn March 24, 2023 19:47
Copy link
Contributor

@mattijn mattijn left a comment

Choose a reason for hiding this comment

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

One question. The text encoding channel is not included within the mark_bar().
Should the base not be just x and y and include the text as part of the mark_text().encode(text='wheat')?

@mcp292
Copy link

mcp292 commented Mar 25, 2023

Personally, I thought that was the cool part: that you define your encoding and then the respective marks either use them in full, or in part.

@joelostblom
Copy link
Contributor Author

To me, there is a conceptual difference between Altair and VegaLite here. In Altair, we can think of it as if it is the Chart object that takes the encoding, and then any number of marks can be added to it, as in the example in this PR:

base = alt.Chart(source).encode(
    x='wheat',
    y="year:O",
    text='wheat'
)
base.mark_bar() + base.mark_text(align='left', dx=1)

However, in VegaLite every mark has to be coupled to its own encoding (I think), so the reason the above spec works is that Altair expands it into a VegaLite spec that contains a redundant text encoding for the bars:

  "layer": [
    {
      "mark": {"type": "bar"},
      "encoding": {
        "text": {"field": "wheat", "type": "quantitative"},
        "x": {"field": "wheat", "type": "quantitative"},
        "y": {"field": "year", "type": "ordinal"}
      }
    },
    {
      "mark": {"type": "text", "align": "left", "dx": 2},
      "encoding": {
        "text": {"field": "wheat", "type": "quantitative"},
        "x": {"field": "wheat", "type": "quantitative"},
        "y": {"field": "year", "type": "ordinal"}
      }
    }
  ],

I believe that being able to think of an encoding as a property of a Chart and that any mark added to that chart can make use of those encodings is a useful level of abstraction to think about the Altair syntax even if it leads to slight increase in the VegaLite verbosity (notably, it is also similar to how users can think of their charts in ggplot and seaborns new api)

@joelostblom
Copy link
Contributor Author

joelostblom commented Mar 27, 2023

Actually, it seems like an encoding can belong to either a single layer or be shared across all layers in Vega-Lite too (by defining it at the Chart/Top level). That means that the conceptual thinking that I described above where an encoding belongs to the overall chart rather than a single mark can apply to Vega-Lite too but it depends on how the code is written (it always applies to Altair). I have broken this out to a separate issue in to discuss more #2999

@mattijn
Copy link
Contributor

mattijn commented Mar 28, 2023

Thanks for the clarification @joelostblom, good that you opened another issue on this topic. Merging this!

@mattijn mattijn merged commit cb5ab75 into master Mar 28, 2023
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.

3 participants