Fix density map in scatter viewer for many points

[astrofrog]

This broke at some point in the last couple of years, have added a visual regression test to make sure it doesn't break again.

I need to try and get the colormap to be correct, I don't think that ever worked but now is a good time to fix it.

Fixes #353

[astrofrog]

@maartenbreddels @mariobuikhuizen - I am running into an issue here where the second visual test fails with:

_______________________________________________________ test_visual_scatter2d_density[chromium] _______________________________________________________

args = ()
kwargs = {'page_session': <Page url='http://0.0.0.0:18865/'>, 'solara_test': None, 'tmp_path': PosixPath('/private/var/folders/zy/t1l3sx310d3d6p0kyxqzlrnr0000gr/T/pytest-of-tom/pytest-50/test_visual_scatter2d_density_0')}

    def wrapper(*args, **kwargs):
>       store.return_value[test_name] = obj(*args, **kwargs)

../../.tox/py311-test-visual/lib/python3.11/site-packages/pytest_mpl/plugin.py:107: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/tests/helpers.py:43: in test_wrapper
    layout = test_function(tmp_path, page_session, *args, **kwargs)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/bqplot/scatter/tests/test_visual.py:59: in test_visual_scatter2d_density
    scatter = app.scatter2d(show=False, data=data1)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/app.py:364: in scatter2d
    view = self.new_data_viewer(viewer_cls, data=data,
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/app.py:190: in new_data_viewer
    viewer = super().new_data_viewer(*args, **kwargs)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue/core/application_base.py:79: in new_data_viewer
    c = viewer_class(self._session, state=state)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/bqplot/scatter/viewer.py:32: in __init__
    super().__init__(*args, **kwargs)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/bqplot/common/viewer.py:90: in __init__
    self.create_layout()
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/view.py:109: in create_layout
    self._layout = layout_factory(self)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/vuetify_layout.py:36: in vuetify_layout_factory
    return LayoutWidget(viewer)
../../.tox/py311-test-visual/lib/python3.11/site-packages/glue_jupyter/vuetify_layout.py:23: in __init__
    super().__init__(*args, **kwargs)
../../.tox/py311-test-visual/lib/python3.11/site-packages/ipyvue/VueTemplateWidget.py:146: in __init__
    super().__init__(*args, **kwargs)
../../.tox/py311-test-visual/lib/python3.11/site-packages/ipywidgets/widgets/widget.py:504: in __init__
    self.open()
../../.tox/py311-test-visual/lib/python3.11/site-packages/ipywidgets/widgets/widget.py:517: in open
    state, buffer_paths, buffers = _remove_buffers(self.get_state())
../../.tox/py311-test-visual/lib/python3.11/site-packages/ipywidgets/widgets/widget.py:615: in get_state
    value = to_json(getattr(self, k), self)
../../.tox/py311-test-visual/lib/python3.11/site-packages/ipywidgets/widgets/widget.py:52: in _widget_to_json
    return "IPY_MODEL_" + x.model_id
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = Layout()

    def model_id_debug(self: ipywidgets.widgets.widget.Widget):
        from ipyvue.ForceLoad import force_load_instance
    
        import solara.comm
    
        if self.comm is None and id(self) in closed_ids and id(self) in closed_stack:
            raise RuntimeError(f"Widget has been closed, the stacktrace when the widget was closed is:\n{closed_stack[id(self)]}")
    
        if self.comm is None or isinstance(self.comm, solara.comm.DummyComm) and force_load_instance.comm is not self.comm:
            stack = solara.comm.orphan_comm_stacks.get(self.comm)
            if stack:
                raise RuntimeError(
                    "Widget has no comm, you are probably using a widget that was created at app startup, the stacktrace when the widget was created is:\n"
                    + stack
                )
            else:
>               raise RuntimeError("Widget has no comm, you are probably using a widget that was closed. The widget is:\n" + repr(self))
E               RuntimeError: Widget has no comm, you are probably using a widget that was closed. The widget is:
E               Layout()

../../.tox/py311-test-visual/lib/python3.11/site-packages/solara/server/patch.py:343: RuntimeError

This doesn't happen if I comment out the first visual test, so it is something to do with having multiple tests and something about the first test presumably changed some state that has a side effect on the second test.

Do you have any ideas what could be going on? You can run the tests locally with: tox -e py311-test-visual

[maartenbreddels]

I made a separate PR #366

I think the reason the 2nd run fails is because that is re-using the widgets

[astrofrog]

Need to make sure we properly implement the dpi and contrast options and update when panning - might want to make an ImageGL subclass which can abstract away some of the things.

[astrofrog]

Well this escalated, but I've now properly implemented the density scatter plotting to be on par with glue-core (this never worked properly in glue-jupyter before)

There are some small bugs I need to iron out, then I'll make sure I add a visual test.

[astrofrog]

@mariobuikhuizen - I have been updating the scatter viewer options to use vuetify instead of ipywidgets, but I'm not super happy at the moment with the vertical spacing of the elements - you can run the following notebook with this branch to see what I mean:

https://gist.github.com/astrofrog/539b17fbe3e3c4a7bbe6d655259a9f4f

The current output looks like:

Ignore the horizontal stretching, this isn't an issue when used in practice as it's just in a sidebar. Would you be able to take a look and see if you can improve the vertical distribution of the components?

In addition, there are a few repeated v-if statements in the .vue file that it might be possible to group together in a nicer way - is that something you could look into?

If you do have time, then feel free to open a PR to my branch - thanks!

[mariobuikhuizen]

@astrofrog I will take a look! 👍

[codecov]

Codecov Report

Patch coverage: 75.86% and project coverage change: -1.70 ⚠️

Comparison is base (aab97d0) 89.01% compared to head (1c3e2d9) 87.32%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #363      +/-   ##
==========================================
- Coverage   89.01%   87.32%   -1.70%     
==========================================
  Files          88       89       +1     
  Lines        4751     4882     +131     
==========================================
+ Hits         4229     4263      +34     
- Misses        522      619      +97     

Impacted Files	Coverage Δ
glue_jupyter/conftest.py	100.00% <ø> (ø)
glue_jupyter/state_traitlets_helpers.py	85.57% <0.00%> (-0.84%)	⬇️
glue_jupyter/bqplot/scatter/tests/test_visual.py	17.02% <8.00%> (-10.26%)	⬇️
...lue_jupyter/bqplot/scatter/scatter_density_mark.py	56.52% <56.52%> (ø)
glue_jupyter/common/state_widgets/layer_scatter.py	86.66% <86.36%> (-13.34%)	⬇️
glue_jupyter/bqplot/scatter/layer_artist.py	90.71% <89.63%> (-9.29%)	⬇️
glue_jupyter/app.py	86.85% <100.00%> (-0.25%)	⬇️
glue_jupyter/bqplot/scatter/tests/test_viewer.py	100.00% <100.00%> (ø)
glue_jupyter/bqplot/tests/test_bqplot.py	100.00% <100.00%> (ø)
...lue_jupyter/common/state_widgets/viewer_scatter.py	100.00% <100.00%> (ø)
... and 2 more

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

[astrofrog]

As it seems to be working fine, I am going to go ahead and merge this.

@mariobuikhuizen - if you have a chance to improve the .vue file, please open a PR directly to glue-jupyter - thanks!

[pllim]

Removing this is not backward compatible and broke Jdaviz.

jdaviz/configs/default/plugins/viewers.py:5: in <module>
    from glue_jupyter.bqplot.scatter.layer_artist import BqplotScatterLayerState

cc @rosteen @kecnry @javerbukh

[kecnry]

Is it safe to just import from glue.viewers.scatter.state import ScatterLayerState instead?

[astrofrog]

Yes but I should indeed fix backward-compatibility!