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

Add method-based attribute setting #2795

Merged
merged 19 commits into from
Jan 3, 2023
Merged

Add method-based attribute setting #2795

merged 19 commits into from
Jan 3, 2023

Conversation

ChristopherDavisUCI
Copy link
Contributor

Based on #1629, #2592, and #2770.

@ChristopherDavisUCI
Copy link
Contributor Author

Hi @mattijn, this cherry-pick procedure was easier than I expected. I think I've brought over all the changes from #2770 except the changes relating to examples and the changes relating to tests. Is it easy for you to make those changes?

@mattijn
Copy link
Contributor

mattijn commented Jan 3, 2023

This PR introduces an alternative method-based syntax for setting encoding channel options.

How will it work

Traditionally, encoding channel options are defined by arguments. For example, below we adjust the y-axis title and increase the step between the x-axis ticks:

import altair as alt
from vega_datasets import data
cars = data.cars()

alt.Chart(cars).mark_point().encode(
    x=alt.X('Horsepower', axis=alt.Axis(tickMinStep=50)),
    y=alt.Y('Miles_per_Gallon', title="Miles per Gallon"),
    color='Origin',
    shape='Origin'
)

In the example above, the axis option of the x channel encoding is set using the axis keyword argument: x=alt.X('Horsepower', axis=alt.Axis(tickMinStep=50)). To define the same X object using the alternative method-based syntax, we can use x=alt.X('Horsepower').axis(tickMinStep=50). In other words, the use of the axis keyword argument is replaced by the use of the axis method.

The same technique works with all encoding channels and all channel options. For example, notice how we make the analogous change with respect to the title option of the y channel. The following produces the same chart as the previous example.

import altair as alt
from vega_datasets import data
cars = data.cars()

alt.Chart(cars).mark_point().encode(
    x=alt.X('Horsepower').axis(tickMinStep=50),
    y=alt.Y('Miles_per_Gallon').title('Miles per Gallon'),
    color='Origin',
    shape='Origin'
)

These option-setter methods can also be chained together, as in the following, in which we set the axis, bin, and scale options of the x channel by using the corresponding methods (axis, bin, and scale). We can break the x definition over multiple lines to improve readability. (This is valid syntax because of the enclosing parentheses from encode.)

 import altair as alt
 from vega_datasets import data
 cars = data.cars()

 alt.Chart(cars).mark_point().encode(
     x=alt.X('Horsepower')
             .axis(ticks=False)
             .bin(maxbins=10)
             .scale(domain=(30,300), reverse=True),
     y=alt.Y('Miles_per_Gallon').title('Miles per Gallon'),
     color='Origin',
     shape='Origin'
 )

Regarding the implementation

  • The method-based attribute setting is implemented using a with_property_setter as defined here, which is added to the class definition in the generate_schema_wrapper.py as decorator as defined here.

  • The autocompletion of these methods is done by using type hinting as defined here, which is added within the class definition as empty functions through usage of the overload decorator as defined here

  • The method-based syntax is well tested and all the examples of the example gallery that contains encoding channel options are fully covered using the method syntax as is shown in this directory.

What are the breaking changes

The following breaking change is for practices that I never seen in occurring in the real application code, but mentioned nevertheless.

  • Getting channel encoding options from encoded channel:

When you have encoded a channel specification (within a chart/mark/encode) eg as such:

x = alt.X(title='my title')

And you want to get the content of the defined parameter title you could do this using the following two approaches:

x.title

or

x['title']

And it would both return

'my title`

After this PR this will still work with x['title'], but when you use x.title it will return something as such:

<altair.utils.schemapi._PropertySetter at 0x10e6af100>

That is a breaking change for getting an encoding channel option.

  • Setting channel encoding options from encoded channel:

When you have a bare encoded channel specification as such:

x = alt.X()

And you like set an encoding channel option on this defined encoded channel. You could use both

x.title = 'my title'

Or:

x['title'] = 'my title'

After this PR this two approaches will continue to be both possible and when called (x) will return both something as such:

X({
  title: 'my title'
})

There is thus no breaking change for setting an encoding channel option.

Limitations on tab-complete for help and docstrings

The docstrings are defined within the property setters class (in the hidden __get__ function, here). This makes it possible for IDEs that are using the ipykernel to show the docstrings:
image

But when working with chaining this only will work with the first chained method. So when you want to use the docstrings or help utility for your second or third channel options, you have to write it as it is the first:
image

Since the help will not appear when pressing shift+tab on the second chained method:
image

We consider this a limitation of the ipykernel for now and raised this issue: ipython/ipykernel#1065

When one is using another python kernel, such as the XPython Raw kernel through xeus-python, it will present the user with the function signature and docstring from the defined type hints:
image

Even for the chained methods after the first method:
image

Since this limitation can be a bit confusing when people start adopting this approach, we first like to collect feedback from users how this is being received before changing all examples to use this new syntax.

Final remarks

I would consider this PR a true team-effort.

Firstly, thanks to @jakevdp for the initial idea and prototype! Thanks to @joelostblom for putting it on the agenda again & your work on the docstrings. Thanks to @ChristopherDavisUCI for implementing the type hinting for autocompletion and documentation. Thanks to @binste for the review on the docs and finally thanks to all other people that have been involved in the comments.

Merging this.

@mattijn mattijn merged commit 88b706f into vega:master Jan 3, 2023
@mattijn mattijn changed the title WIP: Add method-based attribute setting Add method-based attribute setting Jan 3, 2023
@joelostblom
Copy link
Contributor

Wow it is fantastic to see this merged!! 🚀 🥳 Thank you everyone for contributing and working on this over a long period of time 🎆 A true team-effort as Mattijn said above 🙌

@ChristopherDavisUCI
Copy link
Contributor Author

Hi @binste, great to see you adding better support for type hints in #2846. Have you taken a look at the type hints that were added in this PR, for example these lines? I believe those were added in this commit. This was my first time working with type hints, so if you have more experience with them, it would be great if you double-checked that the results produced here look reasonable.

@binste
Copy link
Contributor

binste commented Jan 21, 2023

I can't think of another approach to @overload and your implementation works great in VS Code! :) It's a very helpful addition to the library.

image

I think it's also great that it shows the different function signatures:

image

@ChristopherDavisUCI
Copy link
Contributor Author

Great, thanks a lot @binste, it's very reassuring to have you look it over.

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.

4 participants