-
Notifications
You must be signed in to change notification settings - Fork 39
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 page on magicgui
type registration
#399
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm sorry, when I approved I think I'd only loaded one commit and this PR seemed much smaller... Let me actually go through this sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok left some comments @lucyleeow 🙏
Thank you @DragaDoncila! I've updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lucyleeow sorry, couple more comments. I think this is looking good!
`choices` callable. This callable is either `get_layers_data` or `get_layers`. | ||
These functions retrieve the closest parent `Viewer` of the native | ||
{class}`~magicgui.widgets.bases.CategoricalWidget` widget and returns a list of | ||
{class}`~napari.layers.Layer` or tuple of format ('layer name', `<LayerType>Data`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{class}`~napari.layers.Layer` or tuple of format ('layer name', `<LayerType>Data`). | |
{class}`~napari.layers.Layer` or tuple of format `(layer name, <LayerType>Data)`. |
Thanks @DragaDoncila, changes made! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lucyleeow approved with just two more comments to break up a big paragraph (looks a bit intense on the built docs otherwise)
References and relevant issues
Came about after #68
Details
magicgui
type registration, in particular thereset_choices
connection to layers we do for all widgets that have that method.