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

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

Merged
merged 7 commits into from
Feb 14, 2024

Conversation

ejeschke
Copy link
Owner

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 ejeschke added this to the 5.0 milestone Jan 12, 2024
@ejeschke ejeschke self-assigned this Jan 12, 2024
@github-actions github-actions bot added documentation maintenance Work done to keep code maintained widget labels Jan 12, 2024
@ejeschke ejeschke mentioned this pull request Jan 12, 2024
8 tasks
@ejeschke
Copy link
Owner Author

Rebased

@ejeschke
Copy link
Owner Author

Rebased

@ejeschke
Copy link
Owner Author

@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".

Copy link
Collaborator

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ejeschke
Copy link
Owner Author

ejeschke commented Feb 3, 2024

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
Copy link
Owner Author

ejeschke commented Feb 3, 2024

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

@pllim
Copy link
Collaborator

pllim commented Feb 6, 2024

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
Copy link
Collaborator

pllim commented Feb 6, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

Oooh.. dark mode is kinda cool! 😺

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.

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.

Documentation has been added to the manual.
@ejeschke
Copy link
Owner Author

ejeschke commented Feb 7, 2024

Rebased.

@ejeschke
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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

@ejeschke
Copy link
Owner Author

ejeschke commented Feb 7, 2024

or disable_plugins ... ?

@ejeschke
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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

@pllim
Copy link
Collaborator

pllim commented Feb 7, 2024

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
Copy link
Collaborator

pllim commented Feb 7, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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

@pllim
Copy link
Collaborator

pllim commented Feb 7, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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
Copy link
Collaborator

pllim commented Feb 7, 2024

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

@ejeschke
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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
Copy link
Collaborator

pllim commented Feb 7, 2024

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
Copy link
Collaborator

pllim commented Feb 7, 2024

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
Copy link
Owner Author

ejeschke commented Feb 7, 2024

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

@ejeschke ejeschke merged commit 1c3cd84 into ejeschke:main Feb 14, 2024
10 checks passed
@ejeschke
Copy link
Owner Author

Thanks for your patience and testing, @pllim.

@ejeschke ejeschke deleted the plugin-config branch February 14, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants