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 __or__, __ror__, __ior__ methods to BasePlotlyType to support dict-like updates via pipe operator #4047

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janosh
Copy link
Contributor

@janosh janosh commented Jan 30, 2023

@nicolaskruchten I'm not very familiar with the plotly code base so feel a bit like an elephant in a porcelain shop. But maybe this is first step in the right direction for #4046. Just a rough first draft to solicit feedback at this point. No tests yet.

@alexcjohnson
Copy link
Collaborator

@janosh this looks great, apologies for taking so long to review. One question: does this apply magic underscores? ie if we do things like fig.layout.update(margin_pad=4) do we get {'margin': {'pad': 4}}?

No comments on the code, looks good to me. Just need some tests I guess in test_update_objects, and a changelog entry (after merging master in, which should get the tests passing again).

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.

None yet

2 participants