-
Notifications
You must be signed in to change notification settings - Fork 44
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
Move context_menu_tool to enable.tools namespace #442
Comments
Copied from #441: Yes, this was written when Enaml was more of a thing and it was annoying to have Pyface as a dependency when writing Enaml apps, so we were working to remove it. There's also some gorpiness around the Pyglet/OpenGL and VTK/TVTK backends because we don't have a Pyglet or TVTK backend for Pyface. We have to be a bit careful - I think it is OK for us to have the Wx or Qt backends depending on Pyface as a matter of practicality, but making this part of the public API may break the Pyglet backend, for example. |
@corranwebster Just wanted to double check... when you said "making this part of the public API", did you just mean exposing the tool in |
Yes and no - if importing it (even not in the API) breaks non-Pyface backends, then it would make sense to have some sort of flag that "this requires Pyface". Having a subpackage for these is one clear way to do that. If we went that way, we would probably want to move the hover tool into enable.tools.pyface if it uses Pyface. However I am in the process of writing an issue to open the discussion around how optional Pyface and TraitsUI - it may be that the right decision is instead to go all-in on Pyface and TraitsUI integration and expect all backends to have at least a stub Pyface toolkit. |
Pyglet and VTK backends are gone (#570). Shall we just add the imports from |
Motivated by #441, I guess part of the reasons why it is hard to find
context_menu_tool
is that extrapyface
layer atenable.tools.pyface
, when all the other tools sit at the level ofenable.tools
. Re-exporting it inapi
would help (that is #441).Back when the tool was added in #84, pyface was not a runtime requirement of enable (see this init.py) and I guess the intention then was to keep pyface an optional dependency of enable. But since then pyface has officially become enable runtime dependency, and is imported from plenty more places.
Looks like we can move
context_menu_tool
out toenable.tools
?When we do this, it is important that the original module is kept with import aliases for backward compatibility.
The text was updated successfully, but these errors were encountered: