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

remove duplicate FigureWidget code in graph_objects #3779

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

Conversation

jvdd
Copy link
Contributor

@jvdd jvdd commented Jun 15, 2022

This PR removes the logic that is used to import FigureWidget in __init__.py from graph_objects and imports it from ..graph_objs instead

(I might be missing smth in my understanding - but atm I can't think of a reason why you would want exactly the same code in those two files)

@gvwilson gvwilson changed the title 🧹 remove duplicate FigureWidget code in graph_objects remove duplicate FigureWidget code in graph_objects Aug 12, 2024
@gvwilson gvwilson added P2 considered for next cycle community community contribution fix fixes something broken labels Aug 12, 2024
@gvwilson gvwilson assigned gvwilson and emilykl and unassigned gvwilson Aug 23, 2024
@ndrezn
Copy link
Member

ndrezn commented Aug 23, 2024

This seems like a reasonable change. I'm not sure why we handle FigureWidget differently than any other imports in this file. Python < 3.7 doesn't support .. imports, which is why we have this special handler, but it's not clear to me why we use the special handler for just FigureWidget and no other import.

In favour of this change to make this file easier to understand 👍 I'd be curious whether this breaks compatibility with <3.6 for some arcane reason though (although to be honest I'm open to requiring Python>=3.8 for Plotly.py in general).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution fix fixes something broken P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants