Update choices setter for categorical widget to ensure _default_choices are updated when a callable choices is passed

[jamesyan-git]

Noticed that the choices setter for categorical widgets doesn't update _default_choices, even though _default_choices is set when a callable is passed on init. In our plugin we have to set choices to a callable after widget creation, but we've noticed the combo box doesn't get populated correctly until we force some other event change. This PR fixes that and makes sure reset_choices uses the callable.

I've added a comment and a test that fails on main and passes with this PR.

Let me know if this makes sense or if there's a better way to achieve the desired behaviour! Thanks!

cc @jni

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (57c933c) 89.76% compared to head (70423a7) 89.77%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #563   +/-   ##
=======================================
  Coverage   89.76%   89.77%           
=======================================
  Files          37       37           
  Lines        4437     4438    +1     
=======================================
+ Hits         3983     3984    +1     
  Misses        454      454           

Impacted Files	Coverage Δ
src/magicgui/widgets/bases/_categorical_widget.py	80.55% <100.00%> (+0.27%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jni]

@jamesyan-git so, this kinda sorta makes sense, but it doesn't exactly feel right to me (@tlambert03 please correct me if I'm wrong). It would be good to clarify the motivation a bit more. If I can summarise what happens:

• you use widget_init=my_init_func to set the widget choices in a magic_factory
• This kinda sorta works for a bit but then widget._post_init() gets called, which itself calls self.reset_choices(), and then your setting of the choices gets discarded.
• This PR fixes it because reset_choices() grabs the defaults, which you've now overwritten, so it's happy days.

tbh I must say I don't understand why you would reset_choices in the _post_init. @tlambert03 what's the reason for this?

Either way, @jamesyan-git on our end, can you do widget._default_choices = callable and fix things? If so, I suggest going with that regardless of the outcome here. (@tlambert03 our use case is populating the choices with feature columns from a pandas dataframe in the napari layer metadata, in case you're curious; but we found that we can't get the choices to show up on widget launch.)

[DragaDoncila]

I think the callable was always meant to be used for _default_choices. The first line in the init sets _default_choices=choices without making any checks on it. I think it makes sense for the setter to also update _default_choices when a callable choices is passed. To me looking at this code, it seems like an oversight that it wasn't set as _default_choices to begin with - even the reset_choices docstring mentions that if choices are callable, this function may return different things.

[tlambert03]

yep, this looks great, thanks for the nice PR @jamesyan-git 👍

Sorry in general for the confusing choices behavior. it's long been a target of cleanup (see #306) ... this small change definitely makes sense and was just an oversight