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

select traces doesn't do partial matches on meta #2629

Closed
nicolaskruchten opened this issue Jul 9, 2020 · 16 comments · Fixed by #2836 or #2844
Closed

select traces doesn't do partial matches on meta #2629

nicolaskruchten opened this issue Jul 9, 2020 · 16 comments · Fixed by #2836 or #2844
Assignees
Milestone

Comments

@nicolaskruchten
Copy link
Contributor

Today, the output of

import plotly.graph_objects as go

fig = go.Figure(go.Scatter(meta=dict(a=1,b=2)))

print(len([f for f in fig.select_traces(selector=dict(meta=dict(a=1,b=2)))]))
print(len([f for f in fig.select_traces(selector=dict(meta=dict(a=1)))]))

is 1, 0, meaning that we can't use selector to do partial matches on meta.

It would be really convenient if PX could tag all of its traces with some structured data in meta so that users could target these traces with updates, but for that to be useful we would need to do partial matches.

@jonmmease any major reason not to do this?

@nicolaskruchten nicolaskruchten added this to the 4.10 milestone Jul 9, 2020
@jonmmease
Copy link
Contributor

Does this do what you want?

print(len([f for f in fig.select_traces(selector=dict(meta_a=1))]))

I seem to remember making the design decision that selectors keys needed to be matched in full, and magic underscore notation could be used to target subsets.

Not sure off hand if anything goes wrong if we support partial matches, but it would be a breaking change. We'd also need to decide whether the partial match logic is applied recursively.

@nicolaskruchten
Copy link
Contributor Author

that doesn't work, no, possibly because meta is an open-ended dict with no schema below it?

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-20-9a7a6c9a1903> in <module>
      4 
      5 print(len([f for f in fig.select_traces(selector=dict(meta=dict(a=1,b=2)))]))
----> 6 print(len([f for f in fig.select_traces(selector=dict(meta_a=1))]))

<ipython-input-20-9a7a6c9a1903> in <listcomp>(.0)
      4 
      5 print(len([f for f in fig.select_traces(selector=dict(meta=dict(a=1,b=2)))]))
----> 6 print(len([f for f in fig.select_traces(selector=dict(meta_a=1))]))

~/plotly/plotly.py/packages/python/plotly/plotly/basedatatypes.py in _perform_select_traces(self, filter_by_subplot, grid_subplot_refs, selector)
    805 
    806             # Filter by selector
--> 807             if not self._selector_matches(trace, selector):
    808                 continue
    809 

~/plotly/plotly.py/packages/python/plotly/plotly/basedatatypes.py in _selector_matches(obj, selector)
    816 
    817         for k in selector:
--> 818             if k not in obj:
    819                 return False
    820 

~/plotly/plotly.py/packages/python/plotly/plotly/basedatatypes.py in __contains__(self, prop)
   3907                     return False
   3908             else:
-> 3909                 if obj is not None and p in obj._valid_props:
   3910                     obj = obj[p]
   3911                 else:

AttributeError: 'dict' object has no attribute '_valid_props'

@nicolaskruchten
Copy link
Contributor Author

(to be clear: I would be fine with having this work, it would meet my needs just fine :)

@jonmmease
Copy link
Contributor

Ok, yeah. this is special because meta doesn't have a sub-schema.

I think having special case magic underscore logic to handle dicts is a good approach. This would also be useful in the update_* operations for things like:

fig.update_layout(meta_a=8)

It would mean we're asserting that dict keys are strings without underscores. Not sure at the moment if this should be a special case for meta, or if we would want this to work for any property that has a dict value 🤔 But in either case, I think this would be a backward compatible change to make.

@nicolaskruchten
Copy link
Contributor Author

Or we could have it do partial matches off a dict, as a special case... either way :)

@nicolaskruchten
Copy link
Contributor Author

On further thought: I want to store keys in meta which are user-provided and likely to have underscores in them...

@nicholas-esterer
Copy link
Contributor

nicholas-esterer commented Oct 15, 2020

From reading the dialogue above, I gather that select_traces should allow partial matches using meta no matter if it was made using a dict or the underscore notation. Furthermore, meta_a_b=value should produce meta=dict(a=dict(b=value)) but meta=dict(a_b=another_value) should be allowed, too.
Another thing: do you want another keyword in select_traces, say partial_match which allows turning on or off the functionality of matching subsets of trace parameters?

@nicolaskruchten
Copy link
Contributor Author

I would be OK if partial matches on meta didn't work with magic underscores at all actually, if that's easier to implement.

I don't think I would favour a new partial_match kwarg, no, I think we can just make a special case for meta

@nicholas-esterer
Copy link
Contributor

Ok sounds good.
I don't want to go too far afield here, but what if selector could be a function that returned True if that trace should be selected? The function would be passed the trace. The old dict functionality could be left in, too.

@nicolaskruchten
Copy link
Contributor Author

that's not a bad idea... I could implement my PX stuff on top of that, yes. Is that easier to build?

@nicholas-esterer
Copy link
Contributor

Maybe, I'm looking into it right now 👍

@nicholas-esterer
Copy link
Contributor

Yes I think it would be very easy, basically in packages/python/plotly/plotly/basedatatypes.py you have this function:

def _perform_select_traces(self, filter_by_subplot, grid_subplot_refs, selector): 
    from plotly.subplots import _get_subplot_ref_for_trace                        
                                                                                  
    for trace in self.data:                                                       
        # Filter by subplot                                                       
        if filter_by_subplot:                                                     
            trace_subplot_ref = _get_subplot_ref_for_trace(trace)                 
            if trace_subplot_ref not in grid_subplot_refs:                        
                continue                                                          
                                                                                  
        # Filter by selector                                                      
        if not self._selector_matches(trace, selector):                           
            continue                                                              
                                                                                  
        yield trace                                                               

So I would just add a check to see if selector is a dict, if it is, it calls self._selector_matches, if it's a function it calls that function instead.

@nicholas-esterer
Copy link
Contributor

And if we just leave the dict functionality as it is maybe it avoids breaking code that assumed a full match had to be made?

@nicolaskruchten
Copy link
Contributor Author

OK, let's do that then!

@nicholas-esterer
Copy link
Contributor

Would it be crazy to implement this for all the fig.select_* methods?

@nicolaskruchten
Copy link
Contributor Author

No, seems reasonable and consistent and powerful and flexible to implement it everywhere :)

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