Add dask settings to preferences

[Czaki]

Description

This is the alternative implementation for #4436 PR with using core qt_json_builder features instead of magicgui.

The main advantage of this PR is that it introduces a general mechanism for easily add similar fields in future

Same as in the original PR there are no tests.

Type of change

• Bug-fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

References

How has this been tested?

• example: the test suite for my feature covers cases x, y, and z
• example: all tests pass with my change
• example: I check if my changes works with both PySide and PyQt backends
  as there are small differences between the two Qt bindings.

Final checklist:

• My PR is the minimum possible work for the desired functionality
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• If I included new strings, I have used trans. to make them localizable.
  For more information see our translations guide.

[Czaki]

@jni Which PR one do you prefer? This or #4436?

[codecov]

Codecov Report

Merging #5490 (5335aa9) into main (6d27f81) will decrease coverage by 0.01%.
The diff coverage is 91.17%.

@@            Coverage Diff             @@
##             main    #5490      +/-   ##
==========================================
- Coverage   89.40%   89.40%   -0.01%     
==========================================
  Files         609      609              
  Lines       51146    51178      +32     
==========================================
+ Hits        45729    45755      +26     
- Misses       5417     5423       +6     

Impacted Files	Coverage Δ
napari/_qt/qt_viewer.py	78.97% <85.71%> (+0.14%)	⬆️
napari/settings/_application.py	96.55% <91.66%> (-1.28%)	⬇️
...pari/_qt/dialogs/_tests/test_preferences_dialog.py	100.00% <100.00%> (ø)
napari/_qt/dialogs/preferences_dialog.py	90.17% <100.00%> (+0.36%)	⬆️
napari/components/experimental/chunk/_pool.py	85.71% <0.00%> (-7.94%)	⬇️
napari/components/_tests/test_layer_slicer.py	97.98% <0.00%> (-1.21%)	⬇️
napari/_qt/dialogs/qt_package_installer.py	81.81% <0.00%> (+0.39%)	⬆️
napari/utils/theme.py	94.04% <0.00%> (+0.59%)	⬆️
napari/utils/info.py	85.10% <0.00%> (+1.06%)	⬆️
napari/_qt/__init__.py	56.66% <0.00%> (+6.66%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ppwadhwa]

Hi @Czaki ! Thanks for working on this. It makes sense to me and seems straightforward.
I tried it out and it worked mostly well, except that the dask cache values were not updating properly. I added in a few suggestions to make this work, plus a name change on the update function.

[Czaki]

I decided to workaround in another way. Could you check it now?

[ppwadhwa]

It looks and works great!

[Czaki]

As I bring here your ideas. Did you have idea how to test it properly?

[ppwadhwa]

As I bring here your ideas. Did you have idea how to test it properly?

I will take a look on Monday morning!

[Czaki]

@ppwadhwa any update?

[ppwadhwa]

@Czaki sorry for the delay. I have gotten caught up in other work. I am looking at this now. Thanks for the ping!

[ppwadhwa]

@Czaki Since there are no tests for the json schema builder, what do you think about adding in a test in the napari/_tests folder with a dummy schema. See if that builds the widget? And include a test for a subschema? I can start working on this, but just wondering if you like this idea?

[ppwadhwa]

Another option would be to add a test to the preferences dialog.

[ppwadhwa]

I like the second option better. If you add in the dask field to the dummy settings there, I think its covering the new lines you added in the widget schema builder.

Like this:
In napari/_qt/dialogs/_tests/test_preferences_dialog.py

@pytest.fixture
def pref(qtbot):
    dlg = PreferencesDialog()
    qtbot.addWidget(dlg)
    settings = get_settings()
    assert settings.appearance.theme == 'dark'
    dlg._settings.appearance.theme = 'light'
    assert get_settings().appearance.theme == 'light'
    dask_settings = DaskSettings()
    dask_settings.cache = 4000
    dask_settings.enabled = False
    dlg._settings.application.dask = dask_settings
    yield dlg

[ppwadhwa]

and maybe add a test to make sure it used the HorizontalObjectSchemaWidget?

def test_dask_widget(qtbot, pref):
    assert isinstance(pref._stack.currentWidget().widget.widgets['dask'], HorizontalObjectSchemaWidget)

[Czaki]

and maybe add a test to make sure it used the HorizontalObjectSchemaWidget?

def test_dask_widget(qtbot, pref):
    assert isinstance(pref._stack.currentWidget().widget.widgets['dask'], HorizontalObjectSchemaWidget)

I do not understand why changes in fixture but this test looks enough for me.

[ppwadhwa]

@Czaki you're right -- the fixture isn't needed.

[Czaki]

Without horizontal widget:

[jni]

@Czaki and I had a live discussion on this — thanks @Czaki for walking me through both PRs! Some of the points of discussion:

• @jni: "ok so you are changing a vendored module, which is not so great, has it been changed already?" @Czaki: "yes, and this is only a minor addition that could be considered a bug fix (min/max for spinboxes)." We discussed that we should pay a bit more attention to how we maintain vendored modules (for example, noting the commit hash of the vendored version and keeping a plaintest set of .patches that we applied to that commit). But this should come in later PRs, clearly, and should not be a prerequisite for these improvements.
• @jni: "not sure about the side by side layout, and we should prefer code simplicity to fancy UI." @Czaki: "the design was discussed long ago and I am trying to preserve the design to avoid longer discussions." This is fair enough, and anyway ties into the discussion above about improving vendoring. We should fix in later PRs.
• @jni: "does using qt_json_builder further tie us into Qt in the future? Because we want to eventually have models that are completely separate from Qt and potentially different front-ends, including e.g. a web front end." @Czaki: "No, there are other frontends for json-schema so this approach is actually quite portable."

So I'm happy with where this has landed. Thank you @ppwadhwa for all the initial work here and iteration which led us to this point, 🙏 and thanks @Czaki for picking it up and pushing it over the finish line! 🙏 (And for the live walk-through.)

[goanpeca]

Note: I would like to add that the code vendored is not an actual package, just a repo created by the user. I also talked to him about maintaining it and making it an actual package. This is on my to do list, so that we would not have to vendor it but just install it and extend it. Also as @Czaki this method of using the json schemas is what other tools like Jupyterlab currently use to handle settings and autogenerate the UI from it -> see reference PR jupyterlab/jupyterlab#11079