I’m working on a project that needs lots of toolbars on screen at once, even though not all of them will be used at the same time. So, I’m modelling this ‘foldable’ dock widget after what I remember Photoshop panels used to be like.

It’s a work in progress, but would like to hear constructive suggestions.

https://blocks.programming.dev/0101100101/42c5d67f86c049baa3500aa38e439f8a

  • FizzyOrange@programming.dev
    link
    fedilink
    arrow-up
    3
    ·
    12 days ago

    Easily above average code for Python. I’m going to pick on one method:

    def _set_float_icon(self, is_floating: bool):
                """ set the float icon depending on the status of the parent dock widget """
                if is_floating:
                    self.float_button.setIcon(self.icon_dock)
                else:
                    self.float_button.setIcon(self.icon_float)
    

    First, Python does have ternary expressions so you can

    self.float_button.setIcon(self.icon_dock if is_floating else self.icon_float)
    

    Second, what does this code do?

    foo._set_float_icon(true)
    

    Kind of surprising that it sets the icon to icon_dock right? There are two easy fixes:

    1. Use *, is_floating: bool so you have to name the parameter when you call it.
    2. I’d probably rename it to _update_float_icon() or something.

    Also use Black or Ruff to auto-format your code (it’s pretty well formatted already but those will still improve it and for zero effort).

    • 0101100101@programming.devOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      8 days ago

      Thanks for the compliment! Python isn’t my first language and it’s difficult to be able to switch style from one language to another!

      I always find it difficult to choose when to use ternary statements. Sometimes, for something quick and simple, I will, otherwise I’ll be explicit. This is more of a readability issue than anything else. And I find the ternary statements quite verbose compared to other languages by using the words if/else rather than shorthand symbols.

      You’re absolutely right about the set_float_icon and corresponding method. Coding’s an iterative process and that’s a byproduct. I think set_float_icon() along with complimentary methods like set_docked_icon(), set_minimize_icon(), set_restore_icon() etc may be easier to use / remember wtf it does in six months time!

      Thanks for the black / ruff suggestion. I’ve never heard fo them, but I’m about to go look for them.

    • logging_strict@programming.dev
      link
      fedilink
      arrow-up
      1
      ·
      12 days ago

      ternary expressions

      Recommend against the ternary expression. coverage might not detect the two code blocks. Would also not be able to apply # pragma: no cover comment to a code block that isn’t important enough to justify testing it. Often use do nothing code blocks.

      self.float_button.setIcon(self.icon_dock if is_floating else self.icon_float) lets say while testing something goes wrong and trying to debug what happened.

      Will not see the value that gets past into self.float_button.setIcon

      I like your idea of having the param be keyword only. Makes it more readable.

      • FizzyOrange@programming.dev
        link
        fedilink
        arrow-up
        1
        ·
        11 days ago

        If you branch coverage tool can’t handle branches on the same line I would suggest you use a different one! Does it handle if foo or bar?

        Will not see the value that gets past into self.float_button.setIcon

        Uhm, yes you will? Just step into the function.

        • logging_strict@programming.dev
          link
          fedilink
          arrow-up
          1
          ·
          11 days ago

          self.float_button.setIcon is a Python wrapper around a C++ library. Might not be possible to, or want to, step into the function.

          if foo or bar is part of a if-else condition. The else portion needs # pragma: no cover or coverage may complain.

          • FizzyOrange@programming.dev
            link
            fedilink
            arrow-up
            2
            ·
            9 days ago

            Well… the else condition (bar) needs to be covered. I haven’t used branch coverage tools in Python but in any sane language you cover the actual semantics, not the lines. It shouldn’t make any difference if you write your code on one line, or with ternary expressions, or whatever.

            • logging_strict@programming.dev
              link
              fedilink
              arrow-up
              1
              ·
              9 days ago

              We are dealing with Python and coverage, not some theoretical situation or circumstances.

              Was assuming measuring branch coverage. In which case, my advice is coming from experience

            • logging_strict@programming.dev
              link
              fedilink
              arrow-up
              1
              ·
              9 days ago

              What matters is what the package coverage can do and what limitations or nuances it has. Have to work with what we have. We are lucky to have coverage. Especially within a subprocess and multiprocessing workers

  • dohpaz42@lemmy.world
    link
    fedilink
    English
    arrow-up
    2
    ·
    12 days ago

    Photoshop these days minimizes unused pallets into an icon on a vertical toolbar.

    Something along these lines:

    • 0101100101@programming.devOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      edit-2
      12 days ago

      Thanks. I wondered why I instinctively text-transformed the title of the widget to uppercase! I commented it out thinking perhaps it doesn’t look grammatically correct! Reduced the font size a bit and I think it looks a heck of a lot better now!

  • ulterno@programming.dev
    link
    fedilink
    English
    arrow-up
    2
    ·
    edit-2
    12 days ago

    Nice.

    Depending upon what you are aiming for, I’d go with a sidebar. Something like this:

    Illustrating the idea of widgets in a sidebar

    • (a) The sidebar’s horizontal width can be changed, causing all docked widgets in it to resize their width to fit.
    • (b) The docked widgets/sections may resize vertically, either automatically or manually
      • In case of manual resizing of widgets, you might want to add a scroll area in the widget
    • (c) A widget/section may leave empty area in the bottom or vertically expand to fill
    • (d) In case of empty area, the sidebar can either have empty space or be vertically shortened to expose part of the workspace underneath
    • (e) Instead of having just a single widget, you can have a section on the toolbar, with multiple widgets in it. In this case, when the user changes the tab, the section will either automatically resize (vertically) as per widget requirements, or will stay the size that the user set manually, adding a scrollbar in case of overflow or empty area in case of lack of widget content.

    This is in contrast to usual sidebars that tend to have a main tab bar, which only allows for a single docked widget to be shown at a time. This will allow the user to stack widgets both vertically and horizontally as per their requirements. A similar example can be seen in the right side panel in the Design Mode of Qt Creator itself.
    Folded widgets/sections, when docked, will yield vertical space to other widgets/sections, which will in turn, snap upwards (or you can do downwards if that’s your fancy)

    Maybe you can also make the floating widgets mergeable into tabs, which will reduce the number of point+click actions in cases where only 1 of 2 widgets is being used.


    CC BY-NC-SA 4.0

    • 0101100101@programming.devOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      edit-2
      12 days ago

      Qt automatically handles the conversion of QDockWidgets into tabbed docked widgets when one is dragged over an existing one.

      I have a little demo video, but I have no idea where to upload it to!

      • ulterno@programming.dev
        link
        fedilink
        English
        arrow-up
        0
        ·
        12 days ago

        Oh! You actually used QDockWidget instead of making it from the start. I should have read the code before commenting, I guess.

  • logging_strict@programming.dev
    link
    fedilink
    arrow-up
    3
    arrow-down
    1
    ·
    11 days ago

    Lets fix the Sphinx in-code documentation (Ignoring should never embed classes within other classes)

    class FoldableDockWidget(QDockWidget):
        """
        A simple Qt Widget that adds a 'minimise' button that vertically reduces the dock widget to just the titlebar to
        allow the dock widget to still take up minimal screen real estate
        """
    
        class TitleBarWidget(QWidget):
            def __init__(self, title:str, parent:QWidget=None):
                """
                We create a custom title bar using QWidget as the base. The title bar has to be wrapped in another widget
                so that its background can be styled easier, otherwise it's impossible to style
                :param title: the title to appear in the title bar
                """
    

    Becomes

    class FoldableDockWidget(QDockWidget):
        """
        A simple Qt Widget that adds a 'minimise' button that vertically reduces the dock widget to just the titlebar to
        allow the dock widget to still take up minimal screen real estate
        """
    
        class TitleBarWidget(QWidget):
        """Create a custom title bar using :py:class:`~PySide6.QtWidgets.QWidget` as the base.
        The title bar has to be wrapped in another widget so that its background can be styled
        easier, otherwise it's impossible to style
    
        :ivar title: the title to appear in the title bar
        :vartype title: str
        :ivar parent: Default None. Identify parent widget
        :vartype parent: PySide6.QtWidgets.QWidget | None
        """
    
            def __init__(self, title: str, parent: QWidget = None) -> None:
                """class constructor"""
    

    typing matters. Normally separate typing into stub files. The Sphinx in-code documentation includes typing, so the signatures can be simplified and easier to read

    def __init__(self, title, parent=None):

    • 0101100101@programming.devOP
      link
      fedilink
      English
      arrow-up
      2
      ·
      edit-2
      11 days ago

      Thanks for your response.

      should never embed classes within other classes)

      Why is this? I have to admit that coming from other languages, it feels dirty, but is there a pythonic good reason for this? The class ‘belongs’ to the FoldableDockWidget class, so I figure it’s the best place to put it.

      • logging_strict@programming.dev
        link
        fedilink
        arrow-up
        2
        ·
        11 days ago

        Arguing for modularity. Which isn’t likely in a gist (or a script), but is normal for a package.

        By embedding the class, creates a limitation that prevents abstractions or other implementations of each component. Imagine every suggestion in this conversation thread is another variation with a separate implementation.

        The widget class belongs to the FoldableDockWidget class until it doesn’t. Then a refactor is needed.

        There should be four modules. The entrypoint (and cli options parsing), the application, the dockwidget, and the widget. Each should be testable by itself.

        A widget is not a container. An application is not a container component (avoiding the word widget). Hardwiring a particular implementation of the Windowing Python wrapper is also unnecessary (PySide6). What about PySide2, pyQt5, pyQt6, and whatever else comes next?

        As a side note

        Why is there code in the process guard, besides main() (or a async equivalent)? Only multiprocessing applications have code within the process guard. Code within the process guard is unreachable; can’t be imported. For example, testing just the cli option parsing.

        • 0101100101@programming.devOP
          link
          fedilink
          English
          arrow-up
          1
          ·
          8 days ago

          By embedding the class, creates a limitation that prevents abstractions or other implementations of each component. Imagine every suggestion in this conversation thread is another variation with a separate implementation.

          If the user wanted to create a new FoldableDockWidget with a different title bar, they’d extend the FoldableDockWidget class and override the Titlebar method in their extension of it. I understand your point, but isn’t it over optimisation?

          The widget class belongs to the FoldableDockWidget class until it doesn’t. Then a refactor is needed.

          One line of instantiating code. I can’t imagine where or how the custom title bar would be used outside of the Foldable Dock Widget class though. That’s probably the real reason why I made it a sub class. Not how I’d do it in other languages, but in Python? I’m trying it out!

          Hardwiring a particular implementation of the Windowing python wrapper is necessary. They have slightly different implementations. If something magically new comes along, then, the code is updated. Again, over optimisation here which is unnecessary.

          The code in the process guard is just sample code to demonstrate use of the class. No big deal. It’s separate to the class and not to be imported because… this is a gist of sample code!

          • logging_strict@programming.dev
            link
            fedilink
            arrow-up
            1
            ·
            edit-2
            7 days ago

            Think the quote is premature optimization is the root of all evil. Don’t know who came up with that famous expression.

            In the case of the code in the process guard, perhaps you are right.

            In the case of embedding a class within FoldableDockWidget, it’s simply a case of don’t do that, not optimization.

            Hardwiring a particular implementation of the Windowing python wrapper is necessary.

            qasync

            Python library for using asyncio in Qt-based applications

            ^^ is the package to support them all.

            This comes directly from an app i wrote,

            from qasync import (asyncSlot, QtWidgets, QtGui, QtCore, _make_signaller)

            Here is some code which deals with differences between implementations

            from qasync import (QtWidgets, QtCore, QtGui, QtModuleName)
            
            __all__ = ["QtportQAction", "QtportQScreen", "QtportQScreenImplementation"]
            
            try:
                QtportQAction = QtWidgets.QAction
            except AttributeError as e:
                #"PySide6", "PyQt6"
                QtportQAction = QtGui.QAction
            
            try:
                QtportQScreen = QtWidgets.QDesktopWidget
                QtportQScreenImplementation = "QDesktopWidget"
            except AttributeError as e:
                QtportQScreen = QtGui.QScreen
                QtportQScreenImplementation = "QScreen"
            

            So it can be done!

            In the case of this gist, it’s premature optimization. Generally it’s necessary cuz new implementations come along often.

            • 0101100101@programming.devOP
              link
              fedilink
              English
              arrow-up
              1
              ·
              7 days ago

              In the case of this gist, it’s premature optimization. Generally it’s necessary cuz new implementations come along often.

              That sounds terribly inexperienced. That’s exactly what updates to code are for. You cannot manage all kind of, sort of similar but different libraries with one code base. It would be horrific to even consider it.

              • logging_strict@programming.dev
                link
                fedilink
                arrow-up
                1
                arrow-down
                1
                ·
                edit-2
                6 days ago

                The same argument can be made for supporting Windows and MacOS. Don’t have these dev environments. But somehow found a way to support these platforms.

                If you look into it, pyQt[x] and pySide[x] aren’t all that different. The intent of PySide is to keep them for the most part compatible.

                Don’t have to manage everything, just what is being used.

                Doing the wrong thing explains most my packages:

                wreck – dependency management

                drain-swamp with drain-swamp-action – build backend with build plugins

                logging-strict – strictly validated logging configuration

                pytest-logging-strict – the same thing except a pytest plugin

                What else am i not supposed to do?

  • Yareckt@lemmynsfw.com
    link
    fedilink
    arrow-up
    1
    ·
    edit-2
    12 days ago

    I don’t know anything about qt. But I can criticise the design a bit. If something isn’t possible to realize in qt then I apologise.

    1. The Label may not need to be displayed in a separate line. You maybe could put the basic information abpu the window into the windows title and display additional information from the label when hovering over the title.
    2. The sliders have no way to display what the concrete value selected with the slider is. You may want to include the concrete value next to the slider name.
    3. The 3 buttons in the windows top box could be reduced. From a use case perspective being able to hide the tool window and then restore it at the same position is the same as being able to collapse it.
    4. A kind of spacer between the sliders and the next sliders name could make it faster to identify which slider belongs to which name. Don’t know what would look good but you could try leaving a tiny bit more space between them or including some thin lines that don’t touch the windows edges.
  • logging_strict@programming.dev
    link
    fedilink
    arrow-up
    1
    ·
    edit-2
    11 days ago
    from PySide6.QtWidgets import QDockWidget, QWidget, QToolButton, QStyle, QHBoxLayout, QStyleFactory, QSizePolicy, \
        QApplication, QVBoxLayout, QLabel, QSlider, QMainWindow, QSizeGrip
    

    No code should be more than 80 characters UNLESS it’s unavoidable like with a long regex. Hard limit is normally 88 characters.

    Should be

    from PySide6.QtWidgets import (
        QDockWidget,
        QWidget,
        QToolButton,
        QStyle,
        QHBoxLayout,
        QStyleFactory,
        QSizePolicy,
        QApplication,
        QVBoxLayout,
        QLabel,
        QSlider,
        QMainWindow,
        QSizeGrip,
    )
    

    Or add parenthesis and the last comma. black will reformat it. Then use isort to fix imports ordering.

    Get that you are sharing this as a gist. Normally code like this is broken up into multiple modules so it’s easier on the eyes and less going on.