search

glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
742 stars 153 forks source link

Avoid double messaging on subset creation #2515

Open astrofrog opened 2 months ago

astrofrog commented 2 months ago

Avoid broadcasting a create subset message with an empty subset state followed by an update message, instead just broadcast a create subset message with the correct subset state

astrofrog commented 1 month ago

@dhomeier - would you like to review this? I've had confirmation from the jdaviz side that it works for them.

rosteen commented 1 month ago

Actually, maybe hold off on merging - I think we might need a small PR in Jdaviz, I'm seeing spectral extraction fail to extract when a spatial subset is created. Hopefully a quick fix on our end.

dhomeier commented 1 month ago

But nothing needed on this side, is that right?

rosteen commented 1 month ago

But nothing needed on this side, is that right?

I'm pretty sure that's correct, still investigating the fix on our end.

rosteen commented 1 month ago

@astrofrog It seems like at the point we get the SubsetCreate message, the subset hasn't finished propagating/being applied to all of the data - when I switched to subscribing to SubsetCreate for spectral extraction line 441 in spectral_extraction.py is causing our automatic extraction upon making a subset to fail:

uncertainties = uncert_cube.get_subset_object(
                        subset_id=aperture.selected, cls=StdDevUncertainty
                    )

with the error

File [~/projects/glue/glue/core/data.py:330](http://localhost:8888/lab/tree/~/projects/glue/glue/core/data.py#line=329), in BaseData.get_subset_object(self, subset_id, cls, **kwargs)
    326         raise ValueError('Specify the object class to use with cls= - supported '
    327                          'classes are:' + format_choices(data_translator.supported_classes))
    329 if len(self.subsets) == 0:
--> 330     raise ValueError("Dataset does not contain any subsets")
    331 elif subset_id is None:
    332     if len(self.subsets) == 1:

ValueError: Dataset does not contain any subsets

It seems like this is a timing conflict, would it be possible to send the SubsetCreate message after the subset has propagated to all of the relevant data? Or do you have a workaround you could recommend on our side?

dhomeier commented 1 month ago

The setter is already calling self._broadcast() afaics; any ideas if adding something in BaseData.add_subset could help?