search

ejeschke / ginga

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

Refactor modes into separate modules #984

Closed ejeschke closed 1 year ago

ejeschke commented 2 years ago

This PR refactors the code for all the modes that exist in the ginga.Bindings module into separate modules under ginga.modes. Modes can now be written and understood as having a very similar structure to a plugin--there are explicit constructor, start and stop methods. Mode docstrings can be written and maintained better to document the modes and the code implementing a mode is much easier to understand since it is encapsulated rather than all mixed together in one huge file.

As part of this refactoring, some potentially useful functions are added to the ImageView class, that can be called programmatically for panning by various amounts, rotating by increments, etc.

ejeschke commented 2 years ago

@pllim, do you have time to take a look at this?

pllim commented 2 years ago

With +4,352 −2,572, I don't even know where to begin. Do you have any suggestion on how to effectively vet this since I am unable to look at all the diff?

ejeschke commented 2 years ago

Sorry, @pllim! I realize that maybe this is a lot to digest. It's actually breaking up one large, monolithic module of all the possible user interaction event handling code into smaller, more easily understood modules. But--the API is basically the same. bindings.cfg files are all backward-compatible. There is a bunch of new documentation added in which I tried to clarify the concept of the modes in the viewer widget, and including some new documentation on developing a mode.

Maybe just start by taking a look at the new documentation and let me know what does not seem clear or needs expanding?

pllim commented 2 years ago

Thanks for the clarification! I'll put this in my queue. If this is time sensitive, please give me a deadline.

ejeschke commented 1 year ago

I am unable to go through all the LOC touched here, so I only focused on the user docs, as you suggested. If this is working for your users, feel free to merge.

Thanks for the review, @pllim!