ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
120 stars 77 forks source link

Add a PluginConfig plugin (UI to manage other plugins) #1085

Closed ejeschke closed 4 months ago

ejeschke commented 5 months ago

This adds a global plugin called "PluginConfig" which can be used to easily manage which plugins are enabled/disabled for the Ginga reference viewer. Additionally, it allows the user to configure how the plugins appear under the menus, and some other attributes. It shows up under the "Debug" operations submenu.

Documentation has been added to the manual.

ejeschke commented 5 months ago

Rebased

ejeschke commented 5 months ago

Rebased

ejeschke commented 5 months ago

@pllim, can you try this PR version with stginga? The stginga plugins should appear in the plugin table when you invoke the plugin and all showing "enabled".

ejeschke commented 5 months ago

PluginConfig loaded fine in stginga. But when I Edit on Zoom and set it to autoload. Then I hit Save and quit Ginga. Next time I start Ginga back up (via stginga), Zoom does not autoload. Is that expected behavior?

(I am on Windows 11.)

I do see this in my ~/.ginga/plugins.yml (at the end):

- category: Utils
  enabled: true
  hidden: false
  module: Zoom
  name: Zoom
  ptype: global
  start: true
  workspace: left

@pllim, this worked for me in stginga. My plugins.yml looked similar. I don't think the platform (Windows, etc) should matter.

ejeschke commented 5 months ago

Maybe try on your Linux box at work and see if there is any difference?

pllim commented 5 months ago

p.s. I am not even sure how this happened. I installed this branch on RHEL 8 VM using micromamba as my Python env manager. It is Python 3.12. And somehow Ginga gave itself dark mode. I have never seen this before (in other machines).

Screenshot 2024-02-06 172340
pllim commented 5 months ago

On Linux box, I seem to have the opposite problem. I turn off auto-start but it auto-starts anyway. This is what is in my ~/.ginga/plugins.yml:

- category: Utils
  enabled: true
  hidden: false
  module: Zoom
  name: Zoom
  ptype: global
  start: false
  workspace: left

But if I disable it altogether, then it does not auto-start but then it is also not loaded, so I cannot use it at all in that case.

ejeschke commented 5 months ago

Oooh.. dark mode is kinda cool! :smiley_cat:

Can't claim any credit for any kind of "dark mode". Have no idea why that would happen (ginga doesn't do "themes" or "skins"), sounds like something is a bit wonky with the installation.

ejeschke commented 5 months ago

Rebased.

ejeschke commented 5 months ago

Do you have Zoom listed in your $HOME/.ginga/general.cfg ? For example, in global_plugins?

ejeschke commented 5 months ago

or disable_plugins ... ?

ejeschke commented 5 months ago

And do you have a $HOME/.ginga/ginga_config.py?

pllim commented 5 months ago

Do you have Zoom listed in your $HOME/.ginga/general.cfg ? For example, in global_plugins?

No. There is no global_plugins either in that file.

or disable_plugins ... ?

There is also no disable_plugins in general.cfg.

do you have a $HOME/.ginga/ginga_config.py?

Yes.

pllim commented 5 months ago

Doh... I just noticed this in my $HOME/.ginga/ginga_config.py 🤦 which was last modified on 2022-10-10, which means probably stginga or wss_tools created this before.

def post_gui_config(ginga):
    # Auto start global plugins
    ginga.start_global_plugin('Zoom')
    ginga.start_global_plugin('Header')

So, this makes me wonder how well this new PluginConfig would play with existing config files from older installations. 💭

ejeschke commented 5 months ago

So, this makes me wonder how well this new PluginConfig would play with existing config files from older installations.

Anything done in ginga_config.py is a customized setup. PluginConfig is not designed to deal with that kind of thing. Can you temporarily rename it to test the plugin?

ejeschke commented 5 months ago

I can add something to the docstring help to indicate customization that is not handled, or overrides, the plugin.

pllim commented 5 months ago

Is it too much to ask for the PluginConfig to check if something is being overwritten and if so, update the listing accordingly to indicate as such in the plugin GUI?

ejeschke commented 5 months ago

It would be very difficult in the case of a python file that can do anything. In your particular case shown, the ginga_config is simply starting the plugins regardless of any attributes applied to them. It's not reasonable for this plugin to handle that. Instead, this is a plugin that would allow you to remove your ginga_config and simply set start to True for those two plugins using this new mechanism.

ejeschke commented 5 months ago

I realize that is a change, but this is a major release, and I think it is way cleaner and hopefully more understandable and simpler to use than having to customize a python file.

pllim commented 5 months ago

In that case, should start_global_plugin be deprecated or will that break too many things? 🤔

ejeschke commented 5 months ago

That is how global plugins are started. I think the best thing is to deprecate the idea that one should use ginga_config for things that can be done more cleanly using UI interfaces. I've actually thought about deprecating it (ginga_config), but it can be useful if you want to hack ginga to do some kind of unusual thing that can't be done using the "usual" methods. I can add some language to the manual where it talks about customization using ginga_config.

Is this a file that is written by STScI tools? In that case stginga will continue to function in the usual way since this overrides the PluginConfig mechanism.

pllim commented 5 months ago

Is this a file that is written by STScI tools?

Indeed, it is. See https://github.com/spacetelescope/stginga/blob/master/stginga/examples/configs/ginga_config.py

pllim commented 5 months ago

I think based on our conversations and findings thus far, my conclusion is that while this plugin seems handy for new users of vanilla Ginga, it is less so for downstream packages that might ship their own custom Ginga configurations (on the contrary, it might be confusing). So as long as this does not break any existing config override (like you pointed out), I don't see why it should not be merged. Clearer documentation on such limitations would be very nice indeed. Thanks!

ejeschke commented 5 months ago

Ok, picture becoming clearer. There are several ways to address this. Check out this solution.

ejeschke commented 4 months ago

Thanks for your patience and testing, @pllim.