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

RFC: implement attr function and "magic" underscore behavior for PlotlyDict.update #919

Closed
wants to merge 4 commits into from

Conversation

sglyon
Copy link

@sglyon sglyon commented Jan 19, 2018

This is a request for comments for brining some features of the plotly Julia library over to the python side.

The two main features are:

  1. The ability to use "magic underscore" notation to set nested properties. For example, if I wanted to set marker.line.color to red I could do marker_line_color="red" instead of "marker": {"line": {"color": "red"}}. This is enabled for keyword arguments to the update method of all subclasses of PlotlyDict
  2. A new function attr that allows you to apply the magic underscore notation to groups of nested arguments. For example, if I wanted to set "marker.symbol" to "star" and "marker.line.width" to 3 I could to marker=attr(symbol="star", line_width=3). When I originally wrote this for Julia, the goal was to make it easier to create nested sets of attributes because doing so with Julia's dict notation is cumbersome. Because my goal was convenience in setting attributes, I chose the name attr, but I am 100% open to alternative suggestions for that name.

The semantics of the magic underscore notation are such that if a parent property already exists, its children are not lost when adding another child. (meaning if "marker.symbol" already exists on the trace, then trying to set "marker_line_color" would just add "line.color" within the existing "marker" tree).

Also the code is "smart" about dealing with attributes that have underscores in their name. If you tried to do error_x_color it would create a node error_x with a child node color.

The tests are fairly comprehensive and show how this works.

I'd love to hear what people think and if this is something you want in plotly.py.

If this is something that can be accepted here, I will take the time to update docstrings for PlotlyDict.update to make it clear what the magic underscore notation is as well as document the attr function.

@jackparmer
Copy link
Contributor

It looks like something I would likely use. What do you think @cldougl @chriddyp @jmmease ?

@chriddyp
Copy link
Member

@sglyon - We're planning on bringing https://github.com/jmmease/ipyplotly over to this library which will add some new features to the graph_objs and a much deeper and updated jupyter notebook widget update. This is how it would affect these two features:

The ability to use "magic underscore" notation to set nested properties. For example, if I wanted to set marker.line.color to red I could do marker_line_color="red" instead of "marker": {"line": {"color": "red"}}. This is enabled for keyword arguments to the update method of all subclasses of PlotlyDict

With the new version, nested objects will be settable with dot notation, so:

fig.data[0].marker.line.color = 'red'

will "just work". See the overview notebook for some examples: https://github.com/jmmease/ipyplotly/blob/master/examples/overviews/Overview.ipynb

For example, if I wanted to set "marker.symbol" to "star" and "marker.line.width" to 3 I could to marker=attr(symbol="star", line_width=3)

I'm not sure how this will fit in to the new structure. I'm pretty sure that it won't, that users will need to do either:

fig.data[0].marker.symbol = 'star'
fig.data[0].marker.line.width = 3

or

fig.data[0].marker =dict(symbol='star', line=dict(width=3))

@jonmmease
Copy link
Contributor

I think that in the new paradigm the magic underscore behavior could be supported in the constructors of the various datatypes. I don't see a way to use it to update the properties of already constructed objects in-place (which is fine as far as I'm concerned).

In general, we're still in the very early stages of working through how to bring the ipyplotly datatypes into plotly.py with as little user pain as possible.

@sglyon
Copy link
Author

sglyon commented Jan 23, 2018

Thanks for the feedback.

@chriddyp I might be mis-understanding something, but it seems like the dot notation syntax you mention above is already possible:

In [26]: import plotly.graph_objs as go

In [27]: fig = go.Figure(data=[go.Scatter()])

In [28]: fig.data[0].marker.line.color = "red"

In [29]: fig
Out[29]: {'data': [{'marker': {'line': {'color': 'red'}}, 'type': 'scatter'}]}

@jmmease In the Julia library, the underscore magic is applied to the constructor of each trace type (or a layout). I think this is very natural. I didn't do it here, because some of the tests for downloading a figure from a remote location broke when I turned that on, but that could definitely be resolved.


The innovation here is less in the attr function and more in the underscore notation (without the underscore magic attr has the same behavior as dict). That being said, being able to combine the two, is awesome!

I looked through the Python and Julia docs to find examples of the same figure being generated so we can see usage of these two features. For very simple figures, it doesn't make much of a difference...

# Julia version
function box4()
    x0 = ["day 1", "day 1", "day 1", "day 1", "day 1", "day 1",
          "day 2", "day 2", "day 2", "day 2", "day 2", "day 2"]
    trace1 = box(;y=[0.2, 0.2, 0.6, 1.0, 0.5, 0.4, 0.2, 0.7, 0.9, 0.1, 0.5, 0.3],
                  x=x0,
                  name="kale",
                  marker_color="#3D9970")
    trace2 = box(;y=[0.6, 0.7, 0.3, 0.6, 0.0, 0.5, 0.7, 0.9, 0.5, 0.8, 0.7, 0.2],
                  x=x0,
                  name="radishes",
                  marker_color="#FF4136")
    trace3 = box(;y=[0.1, 0.3, 0.1, 0.9, 0.6, 0.6, 0.9, 1.0, 0.3, 0.6, 0.8, 0.5],
                  x=x0,
                  name="carrots",
                  marker_color="#FF851B")
    data = [trace1, trace2, trace3]
    layout = Layout(;yaxis=attr(title="normalized moisture", zeroline=false),
                    boxmode="group")
    plot(data, layout)
end
# python version
x = ['day 1', 'day 1', 'day 1', 'day 1', 'day 1', 'day 1',
     'day 2', 'day 2', 'day 2', 'day 2', 'day 2', 'day 2']

trace0 = go.Box(
    y=[0.2, 0.2, 0.6, 1.0, 0.5, 0.4, 0.2, 0.7, 0.9, 0.1, 0.5, 0.3],
    x=x,
    name='kale',
    marker=dict(
        color='#3D9970'
    )
)
trace1 = go.Box(
    y=[0.6, 0.7, 0.3, 0.6, 0.0, 0.5, 0.7, 0.9, 0.5, 0.8, 0.7, 0.2],
    x=x,
    name='radishes',
    marker=dict(
        color='#FF4136'
    )
)
trace2 = go.Box(
    y=[0.1, 0.3, 0.1, 0.9, 0.6, 0.6, 0.9, 1.0, 0.3, 0.6, 0.8, 0.5],
    x=x,
    name='carrots',
    marker=dict(
        color='#FF851B'
    )
)
data = [trace0, trace1, trace2]
layout = go.Layout(
    yaxis=dict(
        title='normalized moisture',
        zeroline=False
    ),
    boxmode='group'
)
fig = go.Figure(data=data, layout=layout)

But for more complicated ones, I feel like these features make plotly that much more easy to work with:

# Julia version
function scatter_3d()
    @eval using Distributions
    Σ = fill(0.5, 3, 3) + Diagonal([0.5, 0.5, 0.5])
    obs1 = rand(MvNormal(zeros(3), Σ), 200)'
    obs2 = rand(MvNormal(zeros(3), 0.5Σ), 100)'

    trace1 = scatter3d(;x=obs1[:, 1], y=obs1[:, 2], z=obs1[:, 3],
                        mode="markers", opacity=0.8,
                        marker_size=12, marker_line_width=0.5,
                        marker_line_color="rgba(217, 217, 217, 0.14)")

    trace2 = scatter3d(;x=obs2[:, 1], y=obs2[:, 2], z=obs2[:, 3],
                        mode="markers", opacity=0.9,
                        marker=attr(color="rgb(127, 127, 127)",
                                    symbol="circle", line_width=1.0,
                                    line_color="rgb(204, 204, 204)"))

    layout = Layout(margin=attr(l=0, r=0, t=0, b=0))

    plot([trace1, trace2], layout)
end
# python version
import numpy as np

x, y, z = np.random.multivariate_normal(np.array([0,0,0]), np.eye(3), 200).transpose()
trace1 = go.Scatter3d(
    x=x,
    y=y,
    z=z,
    mode='markers',
    marker=dict(
        size=12,
        line=dict(
            color='rgba(217, 217, 217, 0.14)',
            width=0.5
        ),
        opacity=0.8
    )
)

x2, y2, z2 = np.random.multivariate_normal(np.array([0,0,0]), np.eye(3), 200).transpose()
trace2 = go.Scatter3d(
    x=x2,
    y=y2,
    z=z2,
    mode='markers',
    marker=dict(
        color='rgb(127, 127, 127)',
        size=12,
        symbol='circle',
        line=dict(
            color='rgb(204, 204, 204)',
            width=1
        ),
        opacity=0.9
    )
)
data = [trace1, trace2]
layout = go.Layout(
    margin=dict(
        l=0,
        r=0,
        b=0,
        t=0
    )
)
fig = go.Figure(data=data, layout=layout)

One nice thing about the underscore notation is that it is non-breaking. Any code that works right now on plotly.py should still work -- we just gain a bit of flexibility in how we specify "deep" figure attributes.

@jackparmer
Copy link
Contributor

@empet , do you have any feedback on this new, optional notation?

Since this sounds future proof for ipyplotly, 👍 from me. That was my only potential concern with this.

@empet
Copy link

empet commented Jan 25, 2018

@jackparmer I read this thread since @sglyon opened the PR. The magic underscore seems to be useful when you define traces as instances of different Plotly classes. For more than one year I work exclusively with dicts and don't import plotly.graph_objs. But anyway this magic makes the code cleaner, and I like it.

@sglyon
Copy link
Author

sglyon commented Jan 25, 2018

Great! I'm glad you like this.

I've added docstrings to the new/changed functions so this should be ready for review

cc @jackparmer

@jackparmer
Copy link
Contributor

LGTM! Mind just running tests to make sure we didn't accidentally break anything before we merge?
https://github.com/plotly/plotly.py/blob/master/contributing.md#testing

@chriddyp
Copy link
Member

Mind just running tests to make sure we didn't accidentally break anything before we merge?

I just updated the CircleCI settings to run on forked pull requests. Unfortunately, it looks like this didn't update for existing PRs. Would you mind reopening the PR so that CircleCI runs?

@sglyon
Copy link
Author

sglyon commented Jan 25, 2018

Sure, I did run them locally, but was confused about why CI didn't pick them up.

I'll close and reopen.

@sglyon sglyon closed this Jan 25, 2018
@sglyon sglyon reopened this Jan 25, 2018
@sglyon
Copy link
Author

sglyon commented Jan 25, 2018

Yikes! I'll fix up those tests

@sglyon
Copy link
Author

sglyon commented Jan 25, 2018

... actually. It looks like an issue with the plot schema not being in sync. Any advice on how to fix?

@cldougl
Copy link
Member

cldougl commented Jan 25, 2018

@sglyon for
FAIL: test_default_schema_is_up_to_date you can run make update_default_schema in the root of plotly.py to fix that.

@jackparmer
Copy link
Contributor

This is good-to-go on my end. Last look @cldougl @chriddyp ?

@sglyon
Copy link
Author

sglyon commented Mar 2, 2018

yikes! I did something bad here -- let me clean this up and try again.

@sglyon
Copy link
Author

sglyon commented Mar 2, 2018

OK @jackparmer I've resolved the merge conflicts this had with master. It should be good to merge once CI gives us the green light.

@sglyon
Copy link
Author

sglyon commented Mar 2, 2018

Test failures here don't seem to be my fault. I think master has failing tests (https://circleci.com/gh/plotly/plotly.py/tree/master)

@sglyon
Copy link
Author

sglyon commented Mar 20, 2018

ping! Is anything needed from me to push this through?

@jonmmease
Copy link
Contributor

I was thinking about this some more and I'm wondering why the attr structure is needed. Couldn't all dicts be checked for magic underscore properties?

@jonmmease
Copy link
Contributor

Hi @sglyon ,
A couple of thoughts in case you're interested in picking this back up on top of version 3.

I think it would be great to have the 'magic underscore' behavior supported in the following places in the API:

  1. The constructors of all graph objects: go.Scatter(marker_line_color='blue'), I think this could be implemented in the new BasePlotlyType._process_kwargs method.
  2. The fig.add_* methods for adding traces: fig.add_scatter(..., marker_line_color='blue')
  3. The .update method on graph objects as you had before: scatter.update(marker_line_color='blue')
  4. When processing dicts passed to graph object constructors: go.Scatter(dict(marker_line_color='blue')), or go.Scatter(marker=dict(line_color='blue'))

In terms of the attr syntax, I would still have the question from my previous comment:

I was thinking about this some more and I'm wondering why the attr structure is needed. Couldn't all dicts be checked for magic underscore properties?

@sglyon
Copy link
Author

sglyon commented Jul 20, 2018

Hi @jonmmease

I think you identified the main areas where it makes sense to have this functionality. I would love to see it! That being said, I won't have time, at least not in the next month or two, to get to this myself.

I think the attr syntax is not necessary in this package if we always process dicts using underscore magic.

In this implementation attr(**kwargs) is almost the same dict(**kwargs), except that the underscore magic applies

@jonmmease
Copy link
Contributor

Closing in favor of #1534

@jonmmease jonmmease closed this Apr 23, 2019
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.

6 participants