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

Move context_menu_tool to enable.tools namespace #442

Open
kitchoi opened this issue Nov 11, 2020 · 4 comments
Open

Move context_menu_tool to enable.tools namespace #442

kitchoi opened this issue Nov 11, 2020 · 4 comments

Comments

@kitchoi
Copy link
Contributor

kitchoi commented Nov 11, 2020

Motivated by #441, I guess part of the reasons why it is hard to find context_menu_tool is that extra pyface layer at enable.tools.pyface, when all the other tools sit at the level of enable.tools. Re-exporting it in api 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 to enable.tools?

When we do this, it is important that the original module is kept with import aliases for backward compatibility.

@corranwebster
Copy link
Contributor

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.

@kitchoi
Copy link
Contributor Author

kitchoi commented Nov 11, 2020

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 enable.tools.api? i.e. it is not relevant to moving context_menu_tool out to enable.tools?

@corranwebster
Copy link
Contributor

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.

@jwiggins
Copy link
Member

jwiggins commented Mar 8, 2021

Pyglet and VTK backends are gone (#570). Shall we just add the imports from enable.tools.pyface.api to enable.tools.api?

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

No branches or pull requests

4 participants