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

silx.gui.plot.ImageView: Fixed profile window, added setProfileWindowBehavior method #3457

Merged
merged 8 commits into from
Jun 11, 2021

Conversation

t20100
Copy link
Member

@t20100 t20100 commented May 7, 2021

This PR:

  • restores previous behavior for displaying profile windows (i.e., pop-up)
  • adds ImageView.get|setProfileWindowBehavior method to change it to embed horizontal, vertical and cross profiles on the side widgets.
  • adds ImageView.getProfileToolBar and deprecates ImageView.profile attribute.

closes #3454

Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

  • API looks good.
  • I cannot judge what additional things need to be done when swapping toolbars.
  • Implementation: perhaps make it clear that "popup" is the default by changing
class ImageView(PlotWindow):
    def __init__(self, parent=None, backend=None):
        self.__profile = ProfileToolBar(plot=self)
        self.addToolBar(self.__profile)

to this

class ImageView(PlotWindow):
    def __init__(self, parent=None, backend=None):
        self.__profile = None
        self.setProfileWindowBehavior('popup')

class ImageViewMainWindow(ImageView):
    def __init__(self, parent=None, backend=None):
           self.__profileMenu = None
    
    def __updateProfileMenu(self):
        if self.__profileMenu is None:
            self.__profileMenu = self.menuBar().addMenu('Profile')

Or something like that? Up to you.

@woutdenolf
Copy link
Contributor

woutdenolf commented May 7, 2021

@t20100 I'm still bothered by this toolbar swapping. Would it be possible to have the behavior option in silx.gui.plot.profile._CustomProfileManager so that we don't need a silx.gui.plot.imageview._CustomProfileManager and silx.gui.plot.imageview._ProfileToolBar?

@t20100 t20100 force-pushed the imageview-behavior branch from a6d5ac0 to 1dada63 Compare May 11, 2021 13:48
@t20100
Copy link
Member Author

t20100 commented May 11, 2021

@woutdenolf you're right, even if to me you only want to call setProfileWindowBehavior when creating the widget, but you never know...

Last commit moves the switch of behavior to the silx.gui.plot.ImageView._CustomProfileManager.
The "embedded" behavior is specific to ImageView so to me it cannot really go as is into silx.gui.plot.profile._CustomProfileManager (I can give it a try though).

@t20100
Copy link
Member Author

t20100 commented May 11, 2021

Should be fine now: no longer need for silx.gui.plot.imageview._CustomProfileManager and silx.gui.plot.imageview._ProfileToolBar.

Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

Thanks @t20100 it's better:

  • no swapping of toolbars
  • no extra toolbar and manager classes in ImageView.py
  • explicit default behavior

LGTM

t20100 added a commit to t20100/silx that referenced this pull request May 12, 2021
@vallsv vallsv merged commit b9c30ac into silx-kit:master Jun 11, 2021
@t20100 t20100 deleted the imageview-behavior branch September 1, 2021 14:51
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.

New line profiles in ImageView removed essential functionality for OASYS users
3 participants