Opacity in px.density_mapbox

[tvaucher]

Hello,
This is a fix for #2316 that correctly map the opacity attribute in the trace_path.
It allows to correctly pass opacity as an attribute in px.density_mapbox. Check the issue for reproducible code.

Limitations: I'm not aware of another mapbox figure that would require this change as they use proper markers (scatter, line), but if you are, then please comment and I'll add that.

Thanks for your time.
plotly version=4.5.4

[emmanuelle]

Thank you very much for the fix @tvaucher ! Another possibility would be to use the trace_patch argument of make_figure, as in https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/plotly/express/_chart_types.py#L1403. This would reduce the amount of code in _core.py which is already huge. What do you think @nicolaskruchten ? Anyway we should merge a fix quickly since it concerns a bug so thank you again @tvaucher !

[tvaucher]

Like this then. The if is still necessary as you otherwise end up with markers in the trace_patch which was originally responsable for the ValueError

[emmanuelle]

thank you very much for your reactivity @tvaucher ! @nicolaskruchten you can choose between the two versions :-)

[nicolaskruchten]

Thanks for this PR! Looks like the opacity kwarg is a bit of an odd duck in PX... (see also #2294)

I think what we should do is modify _core.py such that for the specific list of trace types that don't support trace.marker.opacity it explicitly uses trace.opacity instead... this would handle cases like densitymapbox but also pie and funnelarea, so we could undo my changes in #2294. For trace types that do support trace.marker.opacity we should continue doing things as we have been (meaning that px.funnel would need to stop putting opacity in trace_patch).

Thoughts?

[tvaucher]

So revert back to something similar from my first commit like:

if args["opacity"] is None:
    if "barmode" in args and args["barmode"] == "overlay":
        trace_patch["marker"] = dict(opacity=0.5)
elif constructor in [go.Densitymapbox, go.Pie, go.Funnel, go.Funnelarea]:  # add more if existing
    trace_patch["opacity"] = args["opacity"]
else:
    trace_patch["marker"] = dict(opacity=args["opacity"])

As apparently both go.Funnelarea and go.Pie have an opacity property.
Additionally, revert #2294 and remove the opacity attribute from trace_patch(opacity=opacity, ...) in Funnel.

I think it's probably easier to maintain if you add new Figures as you would need anyway to change the trace_patch if it doesn't support marker attributes as you would end up with an if anyway like in my second commit.

If you tell me, I'll make the necessary modifications for your review.

[nicolaskruchten]

Yes, the changes you described above sound great to me! Thank you very much :)

PS: if you rebase onto master now, that CI failure should clear up.

[tvaucher]

Done.

[nicolaskruchten]

Thanks! Something is a bit odd about the rebase though, the diff looks like it includes parts of other pull requests...

[tvaucher]

Yes indeed. Messed a bit with the previous git rebase. It should be much cleaner now (rebased on master and replayed the last 3 commits that contain the real changes).

[nicolaskruchten]

Looks perfect, thanks! I'll do a bit more QA and then merge and it'll be part of the 4.6.0 release :)