search

enthought / enable

Enable: low-level drawing and interaction
Other
93 stars 44 forks source link

Move context_menu_tool to enable.tools namespace #442

Open kitchoi opened 3 years ago

kitchoi commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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