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

The list widget for magicgui/magicfactory does not inherit layout from top level #672

Open
Czaki opened this issue Sep 17, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@Czaki
Copy link
Contributor

Czaki commented Sep 17, 2024

Describe the bug

If create magicgui widget with a vertical layout, the list widget still have horizontal layout.

It led to horizontal stretching of widgets.

To Reproduce

Use function

def combine_channels(channels: list['napari.layers.Image'], mode: CombineMode) -> 'napari.types.LayerDataTuple':
    pass

Expected behavior

New elements should be added in the same layout as main widget.

Screenshots

Screenshot 2024-09-17 at 11 43 02

Environment (please complete the following information):

  • magicgui version 0.6.1

I have opened this issue to discuss how to solve this before open PR.

@Czaki Czaki added the bug Something isn't working label Sep 17, 2024
@tlambert03
Copy link
Member

cc @hanjinliu, could you have a look?

@hanjinliu
Copy link
Collaborator

Hi @Czaki ,
this is not a bug. The layout of a ListEdit is independent of the layout of the parent Container, because they are separate widgets. The default layout of ListEdit is horizontal.
As other widgets, you can configure the layout using Annotated.

@magicgui
def combine_channels(
    channels: Annotated[list['napari.layers.Image'], {"layout": "vertical"}], 
    mode: CombineMode,
) -> 'napari.types.LayerDataTuple':
    pass

@Czaki
Copy link
Contributor Author

Czaki commented Sep 17, 2024

I know that I could change it using annotation. I only claim that it is incorrect behavior to always set the layout to horizontal, not depending on the layout of main widget.

Especially when using magic_factory I could spawn one vertical and one horizontal widget. But could provide only one annotation.

@hanjinliu
Copy link
Collaborator

Switching layout depending on the parent widget confuses the current implementation of magicgui a lot. When the annotation list[T] is to be converted in to a widget, it has to know the layout of the destination parent widget. However, at the time the ListEdit is built, it does not have the destination widget as the parent - this means that we cannot assign layout=None for the default behavior. Besides, the layout of containers cannot be changed after construction. The only way to allow the automatic determination of the layout is to add a if-else statement inside magicgui() that checks the incoming type annotation and determine whether to update the widget options. This implementation will cause inconsistency between @magicgui method and the "direct" method of widget construction - if one manually created a ListEdit and append it to a Container, its behavior may differ from the result of @magicgui.

Eitherway, I don't think there's any "correct" way to say which layout is better. I agree that in your example the layout should be vertical, but there are a lot of different use cases of ListEdit. For example, the sigma parameter of the n-D Gaussian filter accepts either a scalar or an array that matches the number of dimensions. Because a FloatEdit is small, using horizontal layout is more readable in this case. In my opinion, if one wants some rule to automate how to set layout, the rule should be explicitly defined by themselves.

@Czaki
Copy link
Contributor Author

Czaki commented Sep 17, 2024

I do not mean to do it in any container. I mean to do it in function that uses magicgui or magic_factory decorator. We know the layout of parents in such a situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants