diff --git a/glue/core/component.py b/glue/core/component.py index 53b4f52c0..35bd3144c 100644 --- a/glue/core/component.py +++ b/glue/core/component.py @@ -67,11 +67,6 @@ def units(self, value): else: self._units = str(value) - @property - def hidden(self): - """Whether the Component is hidden by default""" - return False - @property def data(self): """ The underlying :class:`numpy.ndarray` """ @@ -250,10 +245,6 @@ def set_parent(self, data): """ Reassign the Data object that this DerivedComponent operates on """ self._data = data - @property - def hidden(self): - return self._link.hidden - @property def data(self): """ Return the numerical data as a numpy array """ diff --git a/glue/core/component_id.py b/glue/core/component_id.py index e05c0b0fe..bdaca9f40 100644 --- a/glue/core/component_id.py +++ b/glue/core/component_id.py @@ -48,13 +48,15 @@ class ComponentID(object): component_id = data.id[name] data[component_id] -> numpy array + + Parameters + ---------- + label : str + Name for the component ID """ - def __init__(self, label, hidden=False, parent=None): - """:param label: Name for the ID - :type label: str""" + def __init__(self, label, parent=None): self._label = str(label) - self._hidden = hidden self.parent = parent # We assign a UUID which can then be used for example in equations # for derived components - the idea is that this doesn't change over @@ -84,15 +86,6 @@ def label(self, value): msg = DataRenameComponentMessage(self.parent, self) self.parent.hub.broadcast(msg) - @property - def hidden(self): - """Whether to hide the component by default""" - return self._hidden - - @hidden.setter - def hidden(self, value): - self._hidden = value - def __str__(self): return str(self._label) @@ -176,6 +169,6 @@ class PixelComponentID(ComponentID): dimensions. """ - def __init__(self, axis, label, hidden=False, parent=None): + def __init__(self, axis, label, parent=None): self.axis = axis - super(PixelComponentID, self).__init__(label, hidden=hidden, parent=parent) + super(PixelComponentID, self).__init__(label, parent=parent) diff --git a/glue/core/component_link.py b/glue/core/component_link.py index 9e98ebbdb..430b26a25 100644 --- a/glue/core/component_link.py +++ b/glue/core/component_link.py @@ -18,6 +18,11 @@ def identity(x): return x + +def null(*args): + return None + + OPSYM = {operator.add: '+', operator.sub: '-', operator.truediv: '/', operator.mul: '*', operator.pow: '**'} @@ -93,7 +98,6 @@ def __init__(self, comp_from, comp_to, using=None, inverse=None, inverse_compone self._using = using self._inverse = inverse - self.hidden = False # show in widgets? self.identity = self._using is identity if not isinstance(comp_from, list): @@ -317,7 +321,6 @@ def __init__(self, comp_from, comp_to, coords, index, pixel2world=True): comp_from = [comp_from[i] for i in self.from_needed] super(CoordinateComponentLink, self).__init__( comp_from, comp_to, self.using) - self.hidden = True def using(self, *args): @@ -382,7 +385,6 @@ def __init__(self, left, right, op): right) to = ComponentID("") - null = lambda *args: None super(BinaryComponentLink, self).__init__(from_, to, null) def replace_ids(self, old, new): @@ -397,13 +399,26 @@ def replace_ids(self, old, new): self._right.replace_ids(old, new) def compute(self, data, view=None): - l = self._left - r = self._right + left = self._left + right = self._right if not isinstance(self._left, numbers.Number): - l = data[self._left, view] + left = data[self._left, view] if not isinstance(self._right, numbers.Number): - r = data[self._right, view] - return self._op(l, r) + right = data[self._right, view] + return self._op(left, right) + + def __gluestate__(self, context): + left = context.id(self._left) + right = context.id(self._right) + operator = context.do(self._op) + return dict(left=left, right=right, operator=operator) + + @classmethod + def __setgluestate__(cls, rec, context): + left = context.object(rec['left']) + right = context.object(rec['right']) + operator = context.object(rec['operator']) + return cls(left, right, operator) def __str__(self): sym = OPSYM.get(self._op, self._op.__name__) diff --git a/glue/core/data.py b/glue/core/data.py index e49c9f864..598083230 100644 --- a/glue/core/data.py +++ b/glue/core/data.py @@ -10,7 +10,8 @@ from glue.core.message import (DataUpdateMessage, DataRemoveComponentMessage, DataAddComponentMessage, NumericalDataChangedMessage, SubsetCreateMessage, ComponentsChangedMessage, - ComponentReplacedMessage, DataReorderComponentMessage) + ComponentReplacedMessage, DataReorderComponentMessage, + ExternallyDerivableComponentsChangedMessage) from glue.core.decorators import clear_cache from glue.core.util import split_component_view from glue.core.hub import Hub @@ -77,6 +78,7 @@ def __init__(self, label="", coords=None, **kwargs): # Components self._components = OrderedDict() + self._externally_derivable_components = OrderedDict() self._pixel_component_ids = ComponentIDList() self._world_component_ids = ComponentIDList() @@ -93,7 +95,7 @@ def __init__(self, label="", coords=None, **kwargs): self.style = VisualAttributes(parent=self) - self._coordinate_links = None + self._coordinate_links = [] self.data = self self.label = label @@ -179,14 +181,29 @@ def remove_component(self, component_id): :param component_id: the component to remove :type component_id: :class:`~glue.core.component_id.ComponentID` """ + # TODO: avoid too many messages when removing a component triggers + # the removal of derived components. if component_id in self._components: self._components.pop(component_id) + self._removed_derived_that_depend_on(component_id) if self.hub: msg = DataRemoveComponentMessage(self, component_id) self.hub.broadcast(msg) msg = ComponentsChangedMessage(self) self.hub.broadcast(msg) + def _removed_derived_that_depend_on(self, component_id): + """ + Remove internal derived components that can no longer be derived. + """ + remove = [] + for cid in self.derived_components: + comp = self.get_component(cid) + if component_id in comp.link.get_from_ids(): + remove.append(cid) + for cid in remove: + self.remove_component(cid) + @contract(other='isinstance(Data)', cid='cid_like', cid_other='cid_like') @@ -359,7 +376,7 @@ def get_component_id(data, name): other._key_joins[self] = (cid_other, cid) @contract(component='component_like', label='cid_like') - def add_component(self, component, label, hidden=False): + def add_component(self, component, label): """ Add a new component to this data set. :param component: object to add. Can be a Component, @@ -385,7 +402,7 @@ def add_component(self, component, label, hidden=False): """ if isinstance(component, ComponentLink): - return self.add_component_link(component, label=label, hidden=hidden) + return self.add_component_link(component, label=label) if not isinstance(component, Component): component = Component.autotyped(component) @@ -405,9 +422,10 @@ def add_component(self, component, label, hidden=False): if component_id.parent is None: component_id.parent = self else: - component_id = ComponentID(label, hidden=hidden, parent=self) + component_id = ComponentID(label, parent=self) if len(self._components) == 0: + # TODO: make sure the following doesn't raise a componentsraised message self._create_pixel_and_world_components(ndim=component.ndim) # In some cases, such as when loading a session, we actually disable the @@ -429,32 +447,62 @@ def add_component(self, component, label, hidden=False): return component_id + def _set_externally_derivable_components(self, derivable_components): + """ + Externally deriable components are components identified by component + IDs from other datasets. + + This method is meant for internal use only and is called by the link + manager. The ``derivable_components`` argument should be set to a + dictionary where the keys are the derivable component IDs, and the + values are DerivedComponent instances which can be used to get the + data. + """ + if len(self._externally_derivable_components) == 0 and len(derivable_components) == 0: + return + self._externally_derivable_components = derivable_components + if self.hub: + msg = ExternallyDerivableComponentsChangedMessage(self) + self.hub.broadcast(msg) + @contract(link=ComponentLink, label='cid_like|None', returns=DerivedComponent) - def add_component_link(self, link, label=None, hidden=False): - """ Shortcut method for generating a new :class:`~glue.core.component.DerivedComponent` - from a ComponentLink object, and adding it to a data set. - - :param link: :class:`~glue.core.component_link.ComponentLink` - :param label: The ComponentID or label to attach to. - :type label: :class:`~glue.core.component_id.ComponentID` or str + def add_component_link(self, link, label=None): + """ + Shortcut method for generating a new + :class:`~glue.core.component.DerivedComponent` from a ComponentLink + object, and adding it to a data set. - :returns: - The :class:`~glue.core.component.DerivedComponent` that was added + Parameters + ---------- + link : :class:`~glue.core.component_link.ComponentLink` + The link to use to generate a new component + label : :class:`~glue.core.component_id.ComponentID` or str + The ComponentID or label to attach to. + + Returns + ------- + component : :class:`~glue.core.component.DerivedComponent` + The component that was added """ if label is not None: if not isinstance(label, ComponentID): - label = ComponentID(label, parent=self, hidden=hidden) + label = ComponentID(label, parent=self) link.set_to_id(label) if link.get_to_id() is None: raise TypeError("Cannot add component_link: " "has no 'to' ComponentID") + for cid in link.get_from_ids(): + if cid not in self.components: + raise ValueError("Can only add internal links with add_component_link " + "- use DataCollection.add_link to add inter-data links") + dc = DerivedComponent(self, link) to_ = link.get_to_id() - self.add_component(dc, label=to_, hidden=hidden) + self.add_component(dc, label=to_) return dc def _create_pixel_and_world_components(self, ndim): @@ -462,15 +510,43 @@ def _create_pixel_and_world_components(self, ndim): for i in range(ndim): comp = CoordinateComponent(self, i) label = pixel_label(i, ndim) - cid = PixelComponentID(i, "Pixel Axis %s" % label, hidden=True, parent=self) + cid = PixelComponentID(i, "Pixel Axis %s" % label, parent=self) self.add_component(comp, cid) self._pixel_component_ids.append(cid) + if self.coords: for i in range(ndim): comp = CoordinateComponent(self, i, world=True) label = self.coords.axis_label(i) - cid = self.add_component(comp, label, hidden=True) + cid = self.add_component(comp, label) self._world_component_ids.append(cid) + self._set_up_coordinate_component_links(ndim) + + def _set_up_coordinate_component_links(self, ndim): + + def make_toworld_func(i): + def pix2world(*args): + return self.coords.pixel2world_single_axis(*args[::-1], axis=ndim - 1 - i) + return pix2world + + def make_topixel_func(i): + def world2pix(*args): + return self.coords.world2pixel_single_axis(*args[::-1], axis=ndim - 1 - i) + return world2pix + + result = [] + for i in range(ndim): + link = CoordinateComponentLink(self._pixel_component_ids, + self._world_component_ids[i], + self.coords, i) + result.append(link) + link = CoordinateComponentLink(self._world_component_ids, + self._pixel_component_ids[i], + self.coords, i, pixel2world=False) + result.append(link) + + self._coordinate_links = result + return result @property def components(self): @@ -480,14 +556,18 @@ def components(self): """ return list(self._components.keys()) + @property + def externally_derivable_components(self): + return list(self._externally_derivable_components.keys()) + @property def visible_components(self): - """ :class:`ComponentIDs ` for all non-hidden components. + """All :class:`ComponentIDs ` in the Data that aren't coordinate. :rtype: list """ return [cid for cid, comp in self._components.items() - if not cid.hidden and not comp.hidden and cid.parent is self] + if not isinstance(comp, CoordinateComponent) and cid.parent is self] @property def coordinate_components(self): @@ -498,6 +578,11 @@ def coordinate_components(self): return [c for c in self.component_ids() if isinstance(self._components[c], CoordinateComponent)] + @property + def main_components(self): + return [c for c in self.component_ids() if + not isinstance(self._components[c], (DerivedComponent, CoordinateComponent))] + @property def primary_components(self): """The ComponentIDs not associated with a :class:`~glue.core.component.DerivedComponent` @@ -545,7 +630,7 @@ def find_component_id(self, label): one takes precedence. """ - for cid_set in (self.primary_components, self.derived_components): + for cid_set in (self.primary_components, self.derived_components, self.coordinate_components, list(self._externally_derivable_components)): result = [] for cid in cid_set: @@ -563,45 +648,27 @@ def find_component_id(self, label): return None @property - def coordinate_links(self): - """A list of the ComponentLinks that connect pixel and - world. If no coordinate transformation object is present, - return an empty list. + def links(self): """ - if self._coordinate_links: - return self._coordinate_links - - if not self.coords: - return [] - - if self.ndim != len(self._pixel_component_ids) or \ - self.ndim != len(self._world_component_ids): - # haven't populated pixel, world coordinates yet - return [] - - def make_toworld_func(i): - def pix2world(*args): - return self.coords.pixel2world_single_axis(*args[::-1], axis=self.ndim - 1 - i) - return pix2world - - def make_topixel_func(i): - def world2pix(*args): - return self.coords.world2pixel_single_axis(*args[::-1], axis=self.ndim - 1 - i) - return world2pix + A list of all the links internal to the dataset. + """ + return self.coordinate_links + self.derived_links - result = [] - for i in range(self.ndim): - link = CoordinateComponentLink(self._pixel_component_ids, - self._world_component_ids[i], - self.coords, i) - result.append(link) - link = CoordinateComponentLink(self._world_component_ids, - self._pixel_component_ids[i], - self.coords, i, pixel2world=False) - result.append(link) + @property + def coordinate_links(self): + """ + A list of the ComponentLinks that connect pixel and world. If no + coordinate transformation object is present, return an empty list. + """ + return self._coordinate_links - self._coordinate_links = result - return result + @property + def derived_links(self): + """ + A list of the links present inside all of the DerivedComponent objects + in this dataset. + """ + return [self.get_component(cid).link for cid in self.derived_components] @contract(axis=int, returns=ComponentID) def get_pixel_component_id(self, axis): @@ -705,7 +772,7 @@ def broadcast(self, attribute): Send a :class:`~glue.core.message.DataUpdateMessage` to the hub :param attribute: Name of an attribute that has changed (or None) - :type attribute: string + :type attribute: str """ if not self.hub: return @@ -756,8 +823,6 @@ def update_id(self, old, new): pass if changed and self.hub is not None: - # promote hidden status - new._hidden = new.hidden and old.hidden # remove old component and broadcast the change # see #508 for discussion of this @@ -768,24 +833,18 @@ def __str__(self): s = "Data Set: %s\n" % self.label s += "Number of dimensions: %i\n" % self.ndim s += "Shape: %s\n" % ' x '.join([str(x) for x in self.shape]) - for hidden in [False, True]: - if hidden: - s += "Hidden " - else: - s += "Main " - s += "components:\n" - if hidden: - components = [c for c in self.components if c not in self.visible_components] - else: - components = [c for c in self.visible_components] - for i, cid in enumerate(components): - if cid.hidden != hidden: - continue - comp = self.get_component(cid) - if comp.units is None or comp.units == '': - s += " %i) %s\n" % (i, cid) - else: - s += " %i) %s [%s]\n" % (i, cid, comp.units) + categories = [('Main', self.main_components), + ('Derived', self.derived_components), + ('Coordinate', self.coordinate_components)] + for category, components in categories: + if len(components) > 0: + s += category + " components:\n" + for cid in components: + comp = self.get_component(cid) + if comp.units is None or comp.units == '': + s += " - {0}\n".format(cid) + else: + s += " - {0} [{1}]\n".format(cid, comp.units) return s[:-1] def __repr__(self): @@ -822,9 +881,11 @@ def __getitem__(self, key): if isinstance(key, ComponentLink): return key.compute(self, view) - try: + if key in self._components: comp = self._components[key] - except KeyError: + elif key in self._externally_derivable_components: + comp = self._externally_derivable_components[key] + else: raise IncompatibleAttribute(key) shp = view_shape(self.shape, view) @@ -859,9 +920,11 @@ def get_component(self, component_id): if isinstance(component_id, six.string_types): component_id = self.id[component_id] - try: + if component_id in self._components: return self._components[component_id] - except KeyError: + elif component_id in self._externally_derivable_components: + return self._externally_derivable_components[component_id] + else: raise IncompatibleAttribute(component_id) def to_dataframe(self, index=None): diff --git a/glue/core/data_collection.py b/glue/core/data_collection.py index bccb9ecbe..f41475367 100644 --- a/glue/core/data_collection.py +++ b/glue/core/data_collection.py @@ -37,7 +37,8 @@ def __init__(self, data=None): :param data: :class:`~glue.core.data.Data` object, or list of such objects """ super(DataCollection, self).__init__() - self._link_manager = LinkManager() + + self._link_manager = LinkManager(self) self._data = [] self.hub = None @@ -76,6 +77,7 @@ def append(self, data): s.register() msg = DataCollectionAddMessage(self, data) self.hub.broadcast(msg) + self._sync_link_manager() def extend(self, data): @@ -117,16 +119,7 @@ def _sync_link_manager(self): # Avoid circular calls with self._no_sync_link_manager(): - - # add any links in the data - for d in self._data: - for derived in d.derived_components: - self._link_manager.add_link(d.get_component(derived).link) - for link in d.coordinate_links: - self._link_manager.add_link(link) - - for d in self._data: - self._link_manager.update_data_components(d) + self._link_manager.update_externally_derivable_components() @contextmanager def _no_sync_link_manager(self): @@ -141,6 +134,13 @@ def links(self): """ return tuple(self._link_manager.links) + @property + def external_links(self): + """ + Tuple of :class:`~glue.core.component_link.ComponentLink` objects. + """ + return tuple(self._link_manager.external_links) + def add_link(self, links): """Add one or more links to the data collection. @@ -151,13 +151,7 @@ def add_link(self, links): :class:`~glue.core.component_link.ComponentLink` instances, or a :class:`~glue.core.link_helpers.LinkCollection` """ - # Adding derived comments can trigger _sync_link_manager but we - # don't need to do this since we are explicitly calling - # update_data_components here. - with self._no_sync_link_manager(): - self._link_manager.add_link(links) - for d in self._data: - self._link_manager.update_data_components(d) + self._link_manager.add_link(links) def remove_link(self, links): """ @@ -170,38 +164,22 @@ def remove_link(self, links): :class:`~glue.core.component_link.ComponentLink` instances, or a :class:`~glue.core.link_helpers.LinkCollection` """ - # Removing derived comments can trigger _sync_link_manager but we - # don't need to do this since we are explicitly calling - # update_data_components here. - with self._no_sync_link_manager(): - self._link_manager.remove_link(links) - for d in self._data: - self._link_manager.update_data_components(d) + self._link_manager.remove_link(links) def _merge_link(self, link): pass def set_links(self, links): - """Override the links in the collection, and update data - objects as necessary. + """ + Override the links in the collection, and update data objects as + necessary. :param links: The new links. An iterable of :class:`~glue.core.component_link.ComponentLink` instances """ + self._link_manager.clear_links() + self._link_manager.add_link(links) - # Adding derived comments can trigger _sync_link_manager but we - # don't need to do this since we are explicitly calling - # update_data_components here. - - with self._no_sync_link_manager(): - - self._link_manager.clear() - - for link in links: - self._link_manager.add_link(link) - - for d in self._data: - self._link_manager.update_data_components(d) def register_to_hub(self, hub): """ Register managed data objects to a hub. diff --git a/glue/core/data_combo_helper.py b/glue/core/data_combo_helper.py index 858d3e1c1..4104a02b2 100644 --- a/glue/core/data_combo_helper.py +++ b/glue/core/data_combo_helper.py @@ -311,8 +311,7 @@ def refresh(self, *args): if self.numeric and self.derived: cids = [ChoiceSeparator('Derived components')] for cid in derived_components: - if not cid.hidden: - cids.append(cid) + cids.append(cid) if len(cids) > 1: choices += cids diff --git a/glue/core/fitters.py b/glue/core/fitters.py index 3a21b7679..e06b05494 100644 --- a/glue/core/fitters.py +++ b/glue/core/fitters.py @@ -149,7 +149,7 @@ def fit(self, x, y, dy, constraints, **options): :type y: :class:`numpy.ndarray` :param dy: 1 sigma uncertainties on each datum (optional) :type dy: :class:`numpy.ndarray` - :param constraints: The current value of :attr:`constraints` + :param constraints: The current value of the ``constraints`` property :param options: kwargs for model hyperparameters. :returns: An object representing the fit result. diff --git a/glue/core/hub.py b/glue/core/hub.py index 2756f0f00..e107983b3 100644 --- a/glue/core/hub.py +++ b/glue/core/hub.py @@ -83,14 +83,12 @@ def subscribe(self, subscriber, message_class, An optional function of the form handler(message) that will receive the message on behalf of the subscriber. If not provided, this defaults to the HubListener's notify method - :type handler: Callable :param filter: An optional function of the form filter(message). Messages are only passed to the subscriber if filter(message) == True. The default is to always pass messages. - :type filter: Callable Raises: diff --git a/glue/core/link_manager.py b/glue/core/link_manager.py index 07ef12580..ecdb4eb0b 100644 --- a/glue/core/link_manager.py +++ b/glue/core/link_manager.py @@ -21,7 +21,7 @@ from glue.external import six from glue.core.hub import HubListener -from glue.core.message import DataCollectionDeleteMessage, ComponentsChangedMessage, DataAddComponentMessage, DataRemoveComponentMessage +from glue.core.message import DataCollectionDeleteMessage, DataRemoveComponentMessage from glue.core.contracts import contract from glue.core.link_helpers import LinkCollection from glue.core.component_link import ComponentLink @@ -51,7 +51,8 @@ def accessible_links(cids, links): def discover_links(data, links): - """ Discover all links to components that can be derived + """ + Discover all links to components that can be derived based on the current components known to a dataset, and a set of ComponentLinks. @@ -131,18 +132,34 @@ class LinkManager(HubListener): and compute which components are accesible from which data sets """ - def __init__(self): - self._links = set() + def __init__(self, data_collection=None): + self._external_links = set() self.hub = None + self.trigger = False + self.data_collection = data_collection def register_to_hub(self, hub): self.hub = hub + self.hub.subscribe(self, DataRemoveComponentMessage, + handler=self._component_removed) self.hub.subscribe(self, DataCollectionDeleteMessage, handler=self._data_removed) + def _component_removed(self, msg): + remove = [] + remove_cid = msg.component_id + for link in self._external_links: + ids = set(link.get_from_ids()) | set([link.get_to_id()]) + for cid in ids: + if cid is remove_cid: + remove.append(link) + break + for link in remove: + self.remove_link(link) + def _data_removed(self, msg): remove = [] - for link in self._links: + for link in self._external_links: ids = set(link.get_from_ids()) | set([link.get_to_id()]) for cid in ids: if cid.parent is msg.data: @@ -151,7 +168,10 @@ def _data_removed(self, msg): for link in remove: self.remove_link(link) - def add_link(self, link): + def clear_links(self): + self._external_links.clear() + + def add_link(self, link, update_external=True): """ Ingest one or more ComponentLinks to the manager @@ -162,24 +182,33 @@ def add_link(self, link): """ if isinstance(link, (LinkCollection, list)): for l in link: - self.add_link(l) + self.add_link(l, update_external=False) + if update_external: + self.update_externally_derivable_components() else: - if link.inverse not in self._links: - self._links.add(link) + if link.inverse not in self._external_links: + self._external_links.add(link) + if update_external: + self.update_externally_derivable_components() @contract(link=ComponentLink) - def remove_link(self, link): + def remove_link(self, link, update_external=True): if isinstance(link, (LinkCollection, list)): for l in link: - self.remove_link(l) + self.remove_link(l, update_external=False) + if update_external: + self.update_externally_derivable_components() else: logging.getLogger(__name__).debug('removing link %s', link) - self._links.remove(link) + self._external_links.remove(link) + if update_external: + self.update_externally_derivable_components() @contract(data=Data) - def update_data_components(self, data): - """Update all the DerivedComponents in a data object, based on - all the Components deriveable based on the links in self. + def update_externally_derivable_components(self, data=None): + """ + Update all the externally derived components in all data objects, based + on all the Components deriveable based on the links in self. This overrides any ComponentLinks stored in the DerivedComponents of the data itself -- any components which @@ -195,62 +224,45 @@ def update_data_components(self, data): DerivedComponents will be replaced / added into the data object """ - if self.hub is None: - self._remove_underiveable_components(data) - self._add_deriveable_components(data) + + if self.data_collection is None: + if data is None: + return + else: + data_collection = [data] else: - before = data.components[:] - with self.hub.ignore_callbacks(DataRemoveComponentMessage): - with self.hub.ignore_callbacks(DataAddComponentMessage): - with self.hub.ignore_callbacks(ComponentsChangedMessage): - self._remove_underiveable_components(data) - self._add_deriveable_components(data) - after = data.components[:] - if len(before) != len(after) or any(before[i] is not after[i] for i in range(len(before))): - msg = ComponentsChangedMessage(data) - self.hub.broadcast(msg) + data_collection = self.data_collection - @property - def _inverse_links(self): - return set(link.inverse for link in self._links if link.inverse is not None) + for data in data_collection: + links = discover_links(data, self._links | self._inverse_links) + comps = {} + for cid, link in six.iteritems(links): + d = DerivedComponent(data, link) + comps[cid] = d + data._set_externally_derivable_components(comps) - def _remove_underiveable_components(self, data): - """ Find and remove any DerivedComponent in the data - which requires a ComponentLink not tracked by this LinkManager - """ - data_links = set(data.get_component(dc).link - for dc in data.derived_components) - missing_links = data_links - self._links - self._inverse_links - to_remove = [] - for m in missing_links: - to_remove.extend(find_dependents(data, m)) - - if getattr(data, 'hub', None) is None: - for r in to_remove: - data.remove_component(r) + @property + def _links(self): + if self.data_collection is None: + data_links = set() else: - with data.hub.delay_callbacks(): - for r in to_remove: - data.remove_component(r) - - def _add_deriveable_components(self, data): - """Find and add any DerivedComponents that a data object can - calculate given the ComponentLinks tracked by this - LinkManager + data_links = set(link for data in self.data_collection for link in data.links) + return data_links | self._external_links - """ - links = discover_links(data, self._links | self._inverse_links) - for cid, link in six.iteritems(links): - d = DerivedComponent(data, link) - if cid not in data.components: - data.add_component(d, cid) + @property + def _inverse_links(self): + return set(link.inverse for link in self._links if link.inverse is not None) @property def links(self): return list(self._links) + @property + def external_links(self): + return list(self._external_links) + def clear(self): - self._links.clear() + self._external_links.clear() def __contains__(self, item): return item in self._links diff --git a/glue/core/message.py b/glue/core/message.py index 3bbda4141..c638f631d 100644 --- a/glue/core/message.py +++ b/glue/core/message.py @@ -156,6 +156,10 @@ def __init__(self, sender, component_ids, tag=None): self.component_ids = component_ids +class ExternallyDerivableComponentsChangedMessage(DataMessage): + pass + + class ComponentsChangedMessage(DataMessage): pass diff --git a/glue/core/simpleforms.py b/glue/core/simpleforms.py index b77a71490..77e691efd 100644 --- a/glue/core/simpleforms.py +++ b/glue/core/simpleforms.py @@ -11,20 +11,20 @@ class Option(object): - """ Base class for other options. This shouldn't be used directly + + Parameters + ---------- + default : object + The default value for this option. + label : str + A short label for this option, to use in the GUI """ def __init__(self, default, label): - """ - :param default: The default value for this option. - :type default: object - :param label: A short label for this option, to use in the GUI - :type label: str - """ self.label = label """A UI label for the setting""" self.default = default @@ -46,22 +46,23 @@ def _validate(self, value): class IntOption(Option): + """ + An integer-valued option. + + Parameters + ---------- + min : int, optional + The minimum valid value + max : int, optional + The maximum valid value + default : int, optional + The default value + label : str, optional + A short label for this option + """ def __init__(self, min=0, max=10, default=1, label="Integer"): - """ - An integer-valued option - - :param min: The minimum valid value - :type min: integer - :param max: The maximum valid value - :type max: integer - :param default: The default value - :type default: integer - :param label: A short label for this option - :type label: str - """ super(IntOption, self).__init__(default, label) - self.min = min self.max = max @@ -84,20 +85,22 @@ def _validate(self, value): class FloatOption(Option): + """ + A floating-point option. + + Parameters + ---------- + min : float, optional + The minimum valid value + max : float, optional + The maximum valid value + default : float, optional + The default value + label : str, optional + A short label for this option + """ def __init__(self, min=0, max=10, default=1, label="Float"): - """ - An floating-point option - - :param min: The minimum valid value - :type min: float - :param max: The maximum valid value - :type max: float - :param default: The default value - :type default: float - :param label: A short label for this option - :type label: str - """ super(FloatOption, self).__init__(default, label) self.min = min self.max = max @@ -112,17 +115,18 @@ def _validate(self, value): class BoolOption(Option): + """ + A boolean-valued option. + + Parameters + ---------- + label : str, optional + A short label for this option + default : bool, optional + The default `True`/`False` value + """ def __init__(self, label="Bool", default=False): - """ - A boolean-valued option - - :param default: The default True/False value - :type default: bool - - :param label: A short label for this option - :type label: str - """ super(BoolOption, self).__init__(default, label) def _validate(self, value): diff --git a/glue/core/state.py b/glue/core/state.py index 21da661ce..05ab56304 100644 --- a/glue/core/state.py +++ b/glue/core/state.py @@ -635,11 +635,66 @@ def _save_data_collection_3(dc, context): return result +@saver(DataCollection, version=4) +def _save_data_collection_4(dc, context): + cids = [c for data in dc for c in data.component_ids()] + components = [data.get_component(c) + for data in dc for c in data.component_ids()] + return dict(data=list(map(context.id, dc)), + links=list(map(context.id, dc.external_links)), + cids=list(map(context.id, cids)), + components=list(map(context.id, components)), + groups=list(map(context.id, dc.subset_groups)), + subset_group_count=dc._sg_count) + + @loader(DataCollection) def _load_data_collection(rec, context): - dc = DataCollection(list(map(context.object, rec['data']))) + + datasets = list(map(context.object, rec['data'])) + links = [context.object(link) for link in rec['links']] - dc.set_links(links) + + # Filter out CoordinateComponentLinks that may have been saved in the past + # as these are now re-generated on-the-fly. + links = [link for link in links if not isinstance(link, CoordinateComponentLink)] + + # Go through and split links into links internal to datasets and ones + # between datasets as this dictates whether they should be set on the + # data collection or on the data objects. + external, internal = [], [] + for link in links: + parent_to = link.get_to_id().parent + for cid in link.get_from_ids(): + if cid.parent is not parent_to: + external.append(link) + break + else: + internal.append(link) + + # Remove components in datasets that have external links + for data in datasets: + remove = [] + for cid in data.derived_components: + comp = data.get_component(cid) + + # Neihter in external nor in links overall + if rec.get('_protocol', 0) <= 3: + if comp.link not in internal: + remove.append(cid) + + if isinstance(comp.link, CoordinateComponentLink): + remove.append(cid) + + if len(comp.link.get_from_ids()) == 1 and comp.link.get_from_ids()[0].parent is comp.link.get_to_id().parent and comp.link.get_from_ids()[0].label == comp.link.get_to_id().label: + remove.append(cid) + + for cid in remove: + data.remove_component(cid) + + dc = DataCollection(datasets) + + dc.set_links(external) coerce_subset_groups(dc) return dc @@ -652,6 +707,7 @@ def _load_data_collection_2(rec, context): grp.register_to_hub(result.hub) return result + @loader(DataCollection, version=3) def _load_data_collection_3(rec, context): result = _load_data_collection_2(rec, context) @@ -659,6 +715,23 @@ def _load_data_collection_3(rec, context): return result +@loader(DataCollection, version=4) +def _load_data_collection_4(rec, context): + + dc = DataCollection(list(map(context.object, rec['data']))) + links = [context.object(link) for link in rec['links']] + dc.set_links(links) + coerce_subset_groups(dc) + + dc._subset_groups = list(map(context.object, rec['groups'])) + for grp in dc.subset_groups: + grp.register_to_hub(dc.hub) + + dc._sg_count = rec['subset_group_count'] + + return dc + + @saver(Data) def _save_data(data, context): @@ -699,8 +772,7 @@ def _load_data(rec, context): # and upgrade it to one. This can be removed once we no longer # support pre-v0.8 session files. if not comp.world and not isinstance(cid, PixelComponentID): - cid = PixelComponentID(comp.axis, cid.label, - hidden=cid.hidden, parent=cid.parent) + cid = PixelComponentID(comp.axis, cid.label, parent=cid.parent) comps[icomp] = (cid, comp) result.add_component(comp, cid) @@ -715,6 +787,9 @@ def _load_data(rec, context): result._world_component_ids = coord[:len(coord) // 2] result._pixel_component_ids = coord[len(coord) // 2:] + # We can now re-generate the coordinate links + result._set_up_coordinate_component_links(result.ndim) + for s in rec['subsets']: result.add_subset(context.object(s)) @@ -797,17 +872,17 @@ def load_cid_tuple(cids): @saver(ComponentID) def _save_component_id(cid, context): - return dict(label=cid.label, hidden=cid.hidden) + return dict(label=cid.label) @loader(ComponentID) def _load_component_id(rec, context): - return ComponentID(rec['label'], rec['hidden']) + return ComponentID(rec['label']) @saver(PixelComponentID) def _save_pixel_component_id(cid, context): - return dict(axis=cid.axis, label=cid.label, hidden=cid.hidden) + return dict(axis=cid.axis, label=cid.label) @loader(PixelComponentID) @@ -816,7 +891,7 @@ def _load_pixel_component_id(rec, context): axis = rec['axis'] else: # backward-compatibility axis = int(rec['label'].split()[2]) - return PixelComponentID(axis, rec['label'], rec['hidden']) + return PixelComponentID(axis, rec['label']) @saver(Component) @@ -883,8 +958,7 @@ def _save_component_link(link, context): to = list(map(context.id, [link.get_to_id()])) using = context.do(link.get_using()) inverse = context.do(link.get_inverse()) - hidden = link.hidden - return dict(frm=frm, to=to, using=using, inverse=inverse, hidden=hidden) + return dict(frm=frm, to=to, using=using, inverse=inverse) @loader(ComponentLink) @@ -894,7 +968,6 @@ def _load_component_link(rec, context): using = context.object(rec['using']) inverse = context.object(rec['inverse']) result = ComponentLink(frm, to, using, inverse) - result.hidden = rec['hidden'] return result @@ -919,6 +992,17 @@ def _load_coordinate_component_link(rec, context): return CoordinateComponentLink(frm, to, coords, index, pix2world) +@saver(types.BuiltinFunctionType) +def _save_builtin_function(function, context): + ref = "%s.%s" % (function.__module__, function.__name__) + return {'function': ref} + + +@loader(types.BuiltinFunctionType) +def _load_builtin_function(rec, context): + return lookup_class_with_patches(rec['function']) + + @saver(types.FunctionType) def _save_function(function, context): ref = "%s.%s" % (function.__module__, function.__name__) diff --git a/glue/core/state_path_patches.txt b/glue/core/state_path_patches.txt index 66b132a09..70c57e5e9 100644 --- a/glue/core/state_path_patches.txt +++ b/glue/core/state_path_patches.txt @@ -24,3 +24,5 @@ glue.viewers.table.qt.viewer_widget.TableWidget -> glue.viewers.table.qt.TableVi glue_vispy_viewers.common.toolbar.PatchedElementSubsetState -> glue.core.subset.ElementSubsetState glue.plugins.dendro_viewer.qt.viewer_widget.DendroWidget -> glue.plugins.dendro_viewer.qt.DendrogramViewer glue.plugins.dendro_viewer.layer_artist.DendroLayerArtist -> glue.plugins.dendro_viewer.layer_artist.DendrogramLayerArtist +builtins.builtin_function_or_method -> types.BuiltinFunctionType +__builtin__.builtin_function_or_method -> types.BuiltinFunctionType diff --git a/glue/core/tests/test_component_link.py b/glue/core/tests/test_component_link.py index 01b302717..6e0eca7e6 100644 --- a/glue/core/tests/test_component_link.py +++ b/glue/core/tests/test_component_link.py @@ -293,30 +293,3 @@ def test_link_str(): assert str(x + x + y) == ('((x + x) + y)') assert repr(x + y) == '' - - -def test_duplicated_links_remove_first_input(): - """ - # test changes introduced for #508 - """ - - d1 = Data(x=[1, 2, 3]) - d2 = Data(y=[2, 4, 6]) - - x = d1.id['x'] - y = d2.id['y'] - - dc = DataCollection([d1, d2]) - - dc.add_link(LinkSame(x, y)) - assert x in d2.components - assert x in d2.components - - # NOTE: the behavior tested here is not desirable anymore, so the relevant - # parts that are no longer true have been commented out and replaced - # by the new behavior. - - # assert y not in d2.components - # assert y not in d1.components - assert y in d2.components - assert y in d1.components diff --git a/glue/core/tests/test_data.py b/glue/core/tests/test_data.py index 19b612006..0be8e63a0 100644 --- a/glue/core/tests/test_data.py +++ b/glue/core/tests/test_data.py @@ -11,7 +11,7 @@ from ..component import Component, DerivedComponent, CategoricalComponent, DateTimeComponent from ..component_id import ComponentID -from ..component_link import ComponentLink +from ..component_link import ComponentLink, CoordinateComponentLink, BinaryComponentLink from ..coordinates import Coordinates from ..data import Data, pixel_label from ..exceptions import IncompatibleAttribute @@ -25,6 +25,7 @@ from .test_state import clone + class _TestCoordinates(Coordinates): def pixel2world(self, *args): @@ -94,7 +95,7 @@ def test_add_component_incompatible_shape(self): assert exc.value.args[0] == ("add_component() missing 1 required " "positional argument: 'label'") else: - assert exc.value.args[0] == ("add_component() takes at least 3 " + assert exc.value.args[0] == ("add_component() takes exactly 3 " "arguments (2 given)") def test_get_getitem_incompatible_attribute(self): @@ -370,6 +371,57 @@ def test_add_derived_implicit(self): # There should be five components: x, y, z, pixel, and world assert len(data.components) == 5 + def test_remove_derived_dependency(self): + + # Regression test for a bug that occurred when removing a component + # used in a derived component, which should also remove the derived + # component itself. To make things more fun, we set up a chain of + # derived components to make sure they are all removed. + + data = Data(a=[1, 2, 3], b=[2, 3, 4], label='data1') + + data['c'] = data.id['a'] + 1 + data['d'] = data.id['c'] + 1 + data['e'] = data.id['d'] + 1 + data['f'] = data.id['e'] + 1 + + a_id = data.id['a'] + b_id = data.id['b'] + c_id = data.id['c'] + d_id = data.id['d'] + e_id = data.id['e'] + f_id = data.id['f'] + + # There should be five components: pixel, world, a, b, c, d, e, f + assert len(data.components) == 8 + + data.remove_component(data.id['d']) + + # This should also remove e and f since they depend on d + + assert len(data.components) == 5 + + assert a_id in data.components + assert b_id in data.components + assert c_id in data.components + assert d_id not in data.components + assert e_id not in data.components + assert f_id not in data.components + + def test_links_property(self): + + data = Data(a=[1, 2, 3], b=[2, 3, 4], label='data1') + + assert len(data.links) == 2 + assert isinstance(data.links[0], CoordinateComponentLink) + assert isinstance(data.links[1], CoordinateComponentLink) + + data['c'] = data.id['a'] + 1 + + assert len(data.links) == 3 + + assert isinstance(data.links[2], BinaryComponentLink) + class TestROICreation(object): @@ -377,13 +429,13 @@ def test_range_roi(self): d = Data(xdata=[1, 2, 3], ydata=[1, 2, 3]) comp = d.get_component(d.id['xdata']) - roi = RangeROI('x', min=2,max=3) + roi = RangeROI('x', min=2, max=3) s = comp.subset_from_roi('xdata', roi) assert isinstance(s, RangeSubsetState) np.testing.assert_array_equal((s.lo, s.hi), [2, 3]) - roi = RangeROI('y', min=2,max=3) + roi = RangeROI('y', min=2, max=3) s = comp.subset_from_roi('xdata', roi, other_att='ydata', other_comp=d.get_component(d.id['ydata'])) assert isinstance(s, RangeSubsetState) @@ -582,11 +634,11 @@ def test_add_binary_component(): Number of dimensions: 1 Shape: 3 Main components: - 0) x - 1) y -Hidden components: - 0) Pixel Axis 0 [x] - 1) World 0 + - x + - y +Coordinate components: + - Pixel Axis 0 [x] + - World 0 """.strip() @@ -596,6 +648,27 @@ def test_data_str(): assert str(d) == EXPECTED_STR +EXPECTED_STR_WITH_DERIVED = """ +Data Set: mydata +Number of dimensions: 1 +Shape: 3 +Main components: + - x + - y +Derived components: + - z +Coordinate components: + - Pixel Axis 0 [x] + - World 0 +""".strip() + + +def test_data_str_with_derived(): + d = Data(x=[1, 2, 3], y=[2, 3, 4], label='mydata') + d['z'] = d.id['x'] + 1 + assert str(d) == EXPECTED_STR_WITH_DERIVED + + def test_update_values_from_data(): d1 = Data(a=[1, 2, 3], b=[4, 5, 6], label='banana') d2 = Data(b=[1, 2, 3, 4], c=[5, 6, 7, 8], label='apple') @@ -665,42 +738,6 @@ def test_find_component_id_with_cid(): assert d1.find_component_id(d1.id['b']) is d1.id['b'] -def test_linked_component_visible(): - - # Regression test for a bug that caused components to become hidden once - # they were linked with another component. - - from ..link_helpers import LinkSame - from ..data_collection import DataCollection - - d1 = Data(x=[1], y=[2]) - d2 = Data(w=[3], v=[4]) - - assert not d1.id['x'].hidden - assert not d2.id['w'].hidden - - dc = DataCollection([d1, d2]) - dc.add_link(LinkSame(d1.id['x'], d2.id['w'])) - - assert d1.id['x'] is d2.id['x'] - assert d1.id['w'] is d2.id['w'] - - assert not d1.id['x'].hidden - assert not d2.id['w'].hidden - - assert not d1.id['w'].hidden - assert not d2.id['x'].hidden - - assert d1.id['x'].parent is d1 - assert d1.id['y'].parent is d1 - - assert d2.id['w'].parent is d2 - assert d2.id['v'].parent is d2 - - assert d1.visible_components == [d1.id['x'], d1.id['y']] - assert d2.visible_components == [d2.id['v'], d2.id['w']] - - def test_parent_preserved_session(): # Regression test for a bug that caused ComponentID parents to not be diff --git a/glue/core/tests/test_data_collection.py b/glue/core/tests/test_data_collection.py index 2ea2b958a..f74d2e694 100644 --- a/glue/core/tests/test_data_collection.py +++ b/glue/core/tests/test_data_collection.py @@ -5,7 +5,7 @@ import pytest import numpy as np from mock import MagicMock -from numpy.testing import assert_array_equal +from numpy.testing import assert_array_equal, assert_equal from ..coordinates import Coordinates from ..component_link import ComponentLink @@ -13,8 +13,11 @@ from ..data_collection import DataCollection from ..hub import HubListener from ..message import (Message, DataCollectionAddMessage, DataRemoveComponentMessage, - DataCollectionDeleteMessage, - ComponentsChangedMessage) + DataCollectionDeleteMessage, DataAddComponentMessage, + ComponentsChangedMessage, ExternallyDerivableComponentsChangedMessage) +from ..exceptions import IncompatibleAttribute + +from .test_state import clone class HubLog(HubListener): @@ -28,6 +31,9 @@ def register_to_hub(self, hub): def notify(self, message): self.messages.append(message) + def clear(self): + self.messages[:] = [] + class TestDataCollection(object): @@ -126,8 +132,10 @@ def test_len(self): assert len(self.dc) == 0 def test_derived_links_autoadd(self): - """When appending a data set, its DerivedComponents - should be ingested into the LinkManager""" + """ + When appending a data set, its DerivedComponents should be ingested into + the LinkManager + """ d = Data() id1 = ComponentID("id1") id2 = ComponentID("id2") @@ -154,18 +162,22 @@ def test_catch_data_add_component_message(self): self.dc.append(d) d.add_component(Component(np.array([1, 2, 3])), id1) - assert not link in self.dc._link_manager + assert link not in self.dc._link_manager + self.log.clear() d.add_component(dc, id2) - msg = self.log.messages[-1] - assert isinstance(msg, ComponentsChangedMessage) assert link in self.dc._link_manager - def test_coordinate_links_auto_added(self): + msgs = sorted(self.log.messages, key=lambda x: str(type(x))) + + assert isinstance(msgs[0], ComponentsChangedMessage) + assert isinstance(msgs[1], DataAddComponentMessage) + + def test_links_auto_added(self): id1 = ComponentID("id1") id2 = ComponentID("id2") link = ComponentLink([id1], id2) - self.data.coordinate_links = [link] + self.data.links = [link] self.dc.append(self.data) assert link in self.dc.links @@ -178,17 +190,18 @@ def test_add_links(self): assert link in self.dc.links def test_add_links_updates_components(self): - """setting links attribute automatically adds components to data""" + """ + Setting links attribute automatically adds components to data + """ d = Data() comp = Component(np.array([1, 2, 3])) id1 = ComponentID("id1") d.add_component(comp, id1) id2 = ComponentID("id2") self.dc.append(d) - link = ComponentLink([id1], id2, using=lambda x: None) - + link = ComponentLink([id1], id2) self.dc.set_links([link]) - assert id2 in d.components + assert_equal(d[id2], d[id1]) def test_links_propagated(self): """Web of links is grown and applied to data automatically""" @@ -200,19 +213,23 @@ def test_links_propagated(self): cid2 = ComponentID('b') cid3 = ComponentID('c') - links1 = ComponentLink([cid1], cid2, lambda x: None) + links1 = ComponentLink([cid1], cid2) dc.add_link(links1) - assert cid2 in d.components - links2 = ComponentLink([cid2], cid3, lambda x: None) + assert_equal(d[cid2], d[cid1]) + + links2 = ComponentLink([cid2], cid3) dc.add_link(links2) - assert cid3 in d.components + assert_equal(d[cid3], d[cid2]) dc.remove_link(links2) - assert cid3 not in d.components + with pytest.raises(IncompatibleAttribute): + d[cid3] + assert_equal(d[cid2], d[cid1]) dc.remove_link(links1) - assert cid2 not in d.components + with pytest.raises(IncompatibleAttribute): + d[cid2] def test_merge_links(self): """Trivial links should be merged, discarding the duplicate ID""" @@ -383,11 +400,38 @@ def test_remove_component_message(self): remove_id = data.id['y'] + self.log.clear() + data.remove_component(remove_id) - msg = self.log.messages[-2] - assert isinstance(msg, DataRemoveComponentMessage) - assert msg.component_id is remove_id + msgs = sorted(self.log.messages, key=lambda x: str(type(x))) - msg = self.log.messages[-1] - assert isinstance(msg, ComponentsChangedMessage) + print([type(msg) for msg in msgs]) + + assert isinstance(msgs[0], ComponentsChangedMessage) + + assert isinstance(msgs[1], DataRemoveComponentMessage) + assert msgs[1].component_id is remove_id + + def test_links_preserved_session(self): + + # This tests that the separation of internal vs external links is + # preserved in session files. + + d1 = Data(a=[1, 2, 3]) + d2 = Data(b=[2, 3, 4]) + + dc = DataCollection([d1, d2]) + dc.add_link(ComponentLink([d2.id['b']], d1.id['a'])) + + d1['x'] = d1.id['a'] + 1 + + assert len(d1.coordinate_links) == 2 + assert len(d1.derived_links) == 1 + assert len(dc._link_manager._external_links) == 1 + + dc2 = clone(dc) + + assert len(dc2[0].coordinate_links) == 2 + assert len(dc2[0].derived_links) == 1 + assert len(dc2._link_manager._external_links) == 1 diff --git a/glue/core/tests/test_link_manager.py b/glue/core/tests/test_link_manager.py index caa5692a6..750d85446 100644 --- a/glue/core/tests/test_link_manager.py +++ b/glue/core/tests/test_link_manager.py @@ -50,7 +50,8 @@ def example_components(self, add_derived=True): self.primary = [c1, c2] self.direct = [c3, c4] - self.derived = [c5, c6] + self.derived = [] + self.externally_derived = [c5, c6] self.inaccessible = [c7, c8] @@ -85,7 +86,7 @@ def test_correct_discover(self): links = discover_links(self.data, self.links) for i in self.inaccessible: - assert not i in links + assert i not in links for d in self.direct: assert d in links @@ -94,7 +95,7 @@ def test_correct_discover(self): assert d in links for p in self.primary: - assert not p in links + assert p not in links def test_links_point_to_proper_ids(self): """ Dictionary values are ComponentLinks which @@ -160,32 +161,33 @@ def test_setup(self): expected = set() assert set(self.data.derived_components) == expected - def test_update_data_components_adds_correctly(self): + def test_update_externally_derivable_components_adds_correctly(self): example_components(self, add_derived=False) lm = LinkManager() list(map(lm.add_link, self.links)) - lm.update_data_components(self.data) - derived = set(self.data.derived_components) - expected = set(self.derived + self.direct) + lm.update_externally_derivable_components(self.data) + derived = set(self.data.externally_derivable_components) + expected = set(self.externally_derived + self.direct) assert derived == expected - def test_update_data_components_removes_correctly(self): + def test_update_externally_derivable_components_removes_correctly(self): # add all but last link to manager example_components(self, add_derived=False) lm = LinkManager() list(map(lm.add_link, self.links[:-1])) + lm.update_externally_derivable_components(self.data) # manually add last link as derived component dc = DerivedComponent(self.data, self.links[-1]) - self.data.add_component(dc, dc.link.get_to_id()) + self.data._externally_derivable_components.update({dc.link.get_to_id(): dc}) removed = set([dc.link.get_to_id()]) - assert dc.link.get_to_id() in self.data.derived_components + assert dc.link.get_to_id() in self.data.externally_derivable_components # this link should be removed upon update_components - lm.update_data_components(self.data) - derived = set(self.data.derived_components) - expected = set(self.direct + self.derived) - removed + lm.update_externally_derivable_components(self.data) + derived = set(self.data.externally_derivable_components) + expected = set(self.direct + self.externally_derived) - removed assert derived == expected def test_derived_links_correctwith_mergers(self): @@ -258,3 +260,43 @@ def test_remove_data_removes_links(self): # Removing dataset should remove related links dc.remove(d1) assert len(dc.links) == 2 + + def test_remove_component_removes_links(self): + + d1 = Data(x=[[1, 2], [3, 4]], label='image') + d2 = Data(a=[1, 2, 3], b=[4, 5, 6], label='catalog') + + dc = DataCollection([d1, d2]) + + assert len(dc.links) == 6 + + assert len(d1.components) == 5 + assert len(d2.components) == 4 + + assert len(d1.externally_derivable_components) == 0 + assert len(d2.externally_derivable_components) == 0 + + dc.add_link(LinkSame(d1.id['x'], d2.id['a'])) + + assert len(dc.links) == 7 + + assert len(d1.components) == 5 + assert len(d2.components) == 4 + + assert len(d1.externally_derivable_components) == 1 + assert len(d2.externally_derivable_components) == 1 + + assert d1.id['x'] in d2.externally_derivable_components + assert d2.id['a'] in d1.externally_derivable_components + + # Removing component a from d2 should remove related links and + # derived components. + d2.remove_component(d2.id['a']) + + assert len(dc.links) == 6 + + assert len(d1.components) == 5 + assert len(d2.components) == 3 + + assert len(d1.externally_derivable_components) == 0 + assert len(d2.externally_derivable_components) == 0 diff --git a/glue/core/tests/test_links.py b/glue/core/tests/test_links.py index 24a06bd2a..191069007 100644 --- a/glue/core/tests/test_links.py +++ b/glue/core/tests/test_links.py @@ -18,7 +18,7 @@ def test_1d_world_link(): dc.add_link(LinkSame(d2.get_world_component_id(0), d1.id['x'])) - assert d2.get_world_component_id(0) in d1.components + assert d2.get_world_component_id(0) in d1.externally_derivable_components np.testing.assert_array_equal(d1[d2.get_world_component_id(0)], x) np.testing.assert_array_equal(d1[d2.get_pixel_component_id(0)], x) diff --git a/glue/core/tests/test_state.py b/glue/core/tests/test_state.py index e4bfa5163..9d209a779 100644 --- a/glue/core/tests/test_state.py +++ b/glue/core/tests/test_state.py @@ -235,6 +235,14 @@ def test_matplotlib_cmap(): assert clone(cm.gist_heat) is cm.gist_heat +def test_binary_component_link(): + d1 = core.Data(x=[1, 2, 3]) + d1['y'] = d1.id['x'] + 1 + assert_equal(d1['y'], [2, 3, 4]) + d2 = clone(d1) + assert_equal(d2['y'], [2, 3, 4]) + + class Spam(object): pass diff --git a/glue/core/util.py b/glue/core/util.py index 07c54f69e..9658ad2c6 100644 --- a/glue/core/util.py +++ b/glue/core/util.py @@ -33,18 +33,21 @@ def relim(lo, hi, log=False): def split_component_view(arg): - """Split the input to data or subset.__getitem__ into its pieces. - - :param arg: The input passed to data or subset.__getitem__. - Assumed to be either a scalar or tuple - - :rtype: tuple - - The first item is the Component selection (a ComponentID or - string) - - The second item is a view (tuple of slices, slice scalar, or view - object) + """ + Split the input to data or subset.__getitem__ into its pieces. + + Parameters + ---------- + arg + The input passed to ``data`` or ``subset.__getitem__``. Assumed to be + either a scalar or tuple + + Returns + ------- + selection + The Component selection (a ComponentID or string) + view + Tuple of slices, slice scalar, or view """ if isinstance(arg, tuple): if len(arg) == 1: @@ -58,14 +61,18 @@ def split_component_view(arg): def join_component_view(component, view): - """Pack a componentID and optional view into single tuple + """ + Pack a ComponentID and optional view into single tuple. - Returns an object compatible with data.__getitem__ and related - methods. Handles edge cases of when view is None, a scalar, a - tuple, etc. + Returns an object compatible with ``data.__getitem__`` and related methods. + Handles edge cases of when view is None, a scalar, a tuple, etc. - :param component: ComponentID - :param view: view into data, or None + Parameters + ---------- + component : `~glue.core.component_id.ComponentID` + The ComponentID to pack + view + The view into the data, or `None` """ if view is None: @@ -81,39 +88,40 @@ def join_component_view(component, view): def facet_subsets(data_collection, cid, lo=None, hi=None, steps=5, prefix='', log=False): - """Create a series of subsets that partition the values of - a particular attribute into several bins - - This creates `steps` new subet groups, adds them to the data collection, - and returns the list of newly created subset groups. - - :param data: DataCollection object to use - :type data: :class:`~glue.core.data_collection.DataCollection` - - :param cid: ComponentID to facet on - :type data: :class:`~glue.core.component_id.ComponentID` - - :param lo: The lower bound for the faceting. Defaults to minimum value - in data - :type lo: float - - :param hi: The upper bound for the faceting. Defaults to maximum - value in data - :type hi: float - - :param steps: The number of subsets to create. Defaults to 5 - :type steps: int - - :param prefix: If present, the new subset labels will begin with `prefix` - :type prefix: str - - :param log: If True, space divisions logarithmically. Default=False - :type log: bool - - :returns: List of :class:`~glue.core.subset_group.SubsetGroup` instances - added to `data` - - Example:: + """ + Create a series of subsets that partition the values of a particular + attribute into several bins + + This creates `steps` new subet groups, adds them to the data collection, and + returns the list of newly created subset groups. + + Parameters + ---------- + data : :class:`~glue.core.data_collection.DataCollection` + The DataCollection object to use + cid : :class:`~glue.core.component_id.ComponentID` + The ComponentID to facet on + lo : float, optional + The lower bound for the faceting. Defaults to minimum value in data + hi : float, optional + The upper bound for the faceting. Defaults to maximum value in data + steps : int, optional + The number of subsets to create. Defaults to 5 + prefix : str, optional + If present, the new subset labels will begin with `prefix` + log : bool, optional + If `True`, space divisions logarithmically. Default is `False` + + Returns + ------- + subset_groups : iterable + List of :class:`~glue.core.subset_group.SubsetGroup` instances added to + `data` + + Examples + -------- + + :: facet_subset(data, data.id['mass'], lo=0, hi=10, steps=2) @@ -132,7 +140,6 @@ def facet_subsets(data_collection, cid, lo=None, hi=None, steps=5, Note that the last range is inclusive on both sides. For example, if ``lo`` is 0 and ``hi`` is 5, and ``steps`` is 5, then the intervals for the subsets are [0,1), [1,2), [2,3), [3,4), and [4,5]. - """ from glue.core.exceptions import IncompatibleAttribute if lo is None or hi is None: @@ -188,16 +195,23 @@ def facet_subsets(data_collection, cid, lo=None, hi=None, steps=5, def colorize_subsets(subsets, cmap, lo=0, hi=1): - """Re-color a list of subsets according to a colormap - - :param subsets: List of subsets - :param cmap: Matplotlib colormap instance - :param lo: Start location in colormap. 0-1. Defaults to 0 - :param hi: End location in colormap. 0-1. Defaults to 1 + """ + Re-color a list of subsets according to a colormap. The colormap will be sampled at `len(subsets)` even intervals between `lo` and `hi`. The color at the `ith` interval will be applied to `subsets[i]` + + Parameters + ---------- + subsets : list + List of subsets + cmap : `~matplotlib.colors.Colormap` + Matplotlib colormap instance + lo : float, optional + Start location in colormap. 0-1. Defaults to 0 + hi : float, optional + End location in colormap. 0-1. Defaults to 1 """ from matplotlib import cm @@ -217,13 +231,18 @@ def colorize_subsets(subsets, cmap, lo=0, hi=1): def disambiguate(label, taken): - """If necessary, add a suffix to label to avoid name conflicts + """ + If necessary, add a suffix to label to avoid name conflicts - :param label: desired label - :param taken: set of taken names + Returns label if it is not in the taken set. Otherwise, returns label_NN + where NN is the lowest integer such that label_NN not in taken. - Returns label if it is not in the taken set. Otherwise, returns - label_NN where NN is the lowest integer such that label_NN not in taken. + Parameters + ---------- + label : str + Desired label + taken : iterable + The set of already taken names """ if label not in taken: return label @@ -239,12 +258,18 @@ def row_lookup(data, categories): """ Lookup which row in categories each data item is equal to - :param data: array-like - :param categories: array-like of unique values - - :returns: Float array. - If result[i] is finite, then data[i] = categoreis[result[i]] - Otherwise, data[i] is not in the categories list + Parameters + ---------- + data + An array-like object + categories + Array-like of unique values + + Returns + ------- + array + If result[i] is finite, then data[i] = categoreis[result[i]] + Otherwise, data[i] is not in the categories list """ # np.searchsorted doesn't work on mixed types in Python3 @@ -262,8 +287,7 @@ def row_lookup(data, categories): def small_view(data, attribute): """ - Extract a downsampled view from a dataset, for quick - statistical summaries + Extract a downsampled view from a dataset, for quick statistical summaries """ shp = data.shape view = tuple([slice(None, None, np.intp(max(s / 50, 1))) for s in shp]) @@ -290,8 +314,12 @@ def visible_limits(artists, axis): Returns a tuple of min, max for the requested axis, or None if no data present - :param artists: An iterable collection of artists - :param axis: Which axis to compute. 0=xaxis, 1=yaxis + Parameters + ---------- + artists : iterable + An iterable collection of artists + axis : int + Which axis to compute. 0=xaxis, 1=yaxis """ data = [] for art in artists: @@ -339,12 +367,18 @@ def update_ticks(axes, coord, components, is_log): Changes the axes to have the proper tick formatting based on the type of component. - :param axes: A matplotlib axis object to alter - :param coord: 'x' or 'y' - :param components: A list() of components that are plotted along this axis - :param is_log: Boolean for log-scale. - :kwarg max_categories: The maximum number of categories to display. - :return: None or #categories if components is Categorical + Returns `None` or the number of categories if components is Categorical. + + Parameters + ---------- + axes : `~matplotlib.axes.Axes` + A matplotlib axis object to alter + coord : { 'x' | 'y' } + The coordinate axis on which to update the ticks + components : iterable + A list of components that are plotted along this axis + if_log : boolean + Whether the axis has a log-scale """ if coord == 'x': diff --git a/glue/dialogs/component_manager/qt/component_manager.py b/glue/dialogs/component_manager/qt/component_manager.py index 5088b3ebe..f3edaba1f 100644 --- a/glue/dialogs/component_manager/qt/component_manager.py +++ b/glue/dialogs/component_manager/qt/component_manager.py @@ -88,13 +88,12 @@ def __init__(self, data_collection=None, parent=None): for data in data_collection: - for cid in data.primary_components: - if not cid.hidden: - comp_state = {} - comp_state['cid'] = cid - comp_state['label'] = cid.label - self._state[data][cid] = comp_state - self._components[data]['main'].append(cid) + for cid in data.main_components: + comp_state = {} + comp_state['cid'] = cid + comp_state['label'] = cid.label + self._state[data][cid] = comp_state + self._components[data]['main'].append(cid) self._components[data]['derived'] = [] @@ -108,6 +107,17 @@ def __init__(self, data_collection=None, parent=None): self._state[data][cid] = comp_state self._components[data]['derived'].append(cid) + self._components[data]['other'] = [] + + handled_components = (data.pixel_component_ids + + data.world_component_ids + + self._components[data]['main'] + + self._components[data]['derived']) + + for cid in data.components: + if cid not in handled_components: + self._components[data]['other'].append(cid) + # Populate data combo for data in self.data_collection: self.ui.combosel_data.addItem(data.label, userData=data) @@ -302,6 +312,7 @@ def accept(self): cids_main = self._components[data]['main'] cids_derived = self._components[data]['derived'] + cids_other = self._components[data]['other'] # First deal with renaming of components for cid_new in cids_main + cids_derived: @@ -309,7 +320,7 @@ def accept(self): if label != cid_new.label: cid_new.label = label - cids_all = data.pixel_component_ids + data.world_component_ids + cids_main + cids_derived + cids_all = data.pixel_component_ids + data.world_component_ids + cids_main + cids_derived + cids_other cids_existing = data.components diff --git a/glue/dialogs/component_manager/qt/tests/test_component_manager.py b/glue/dialogs/component_manager/qt/tests/test_component_manager.py index ae387f577..79733ed49 100644 --- a/glue/dialogs/component_manager/qt/tests/test_component_manager.py +++ b/glue/dialogs/component_manager/qt/tests/test_component_manager.py @@ -7,6 +7,7 @@ from glue.core import Data, DataCollection, HubListener, ComponentID from glue.core import message as msg from glue.utils.qt import get_qapp +from glue.core.component_link import ComponentLink from glue.core.parse import ParsedCommand, ParsedComponentLink from ..component_manager import ComponentManagerWidget, EquationEditorDialog @@ -86,14 +87,16 @@ def assert_exact_changes(self, added=[], removed=[], renamed=[], reordered=[], n assert set(added) == set(self.added) assert set(removed) == set(self.removed) assert set(renamed) == set(self.renamed) - assert set(reordered) == set(self.reordered) + assert reordered == self.reordered assert numerical is self.numerical class TestComponentManagerWidget: def setup_method(self): + self.app = get_qapp() + self.data1 = Data(x=[1, 2, 3], y=[3.5, 4.5, -1.0], z=['a', 'r', 'w']) self.data2 = Data(a=[3, 4, 1], b=[1.5, -2.0, 3.5], c=['y', 'e', 'r']) @@ -104,6 +107,10 @@ def setup_method(self): self.data2.add_component_link(link) self.data_collection = DataCollection([self.data1, self.data2]) + + link = ComponentLink([self.data1.id['x']], self.data2.id['a']) + self.data_collection.add_link(link) + self.listener1 = ChangeListener(self.data1) self.listener2 = ChangeListener(self.data2) diff --git a/glue/dialogs/link_editor/qt/link_editor.py b/glue/dialogs/link_editor/qt/link_editor.py index 0405feed4..1696ca07e 100644 --- a/glue/dialogs/link_editor/qt/link_editor.py +++ b/glue/dialogs/link_editor/qt/link_editor.py @@ -106,7 +106,7 @@ def _add_new_link(self): def links(self): current = self._ui.current_links - return current.data.values() + return list(current.data.values()) def _remove_link(self): current = self._ui.current_links diff --git a/glue/viewers/common/qt/data_viewer_with_state.py b/glue/viewers/common/qt/data_viewer_with_state.py index ea2acc511..78864a4ed 100644 --- a/glue/viewers/common/qt/data_viewer_with_state.py +++ b/glue/viewers/common/qt/data_viewer_with_state.py @@ -201,6 +201,10 @@ def register_to_hub(self, hub): handler=self._update_data, filter=self._has_data_or_subset) + hub.subscribe(self, msg.ExternallyDerivableComponentsChangedMessage, + handler=self._update_data, + filter=self._has_data_or_subset) + hub.subscribe(self, msg.SettingsChangeMessage, self._update_appearance_from_settings, filter=self._is_appearance_settings) diff --git a/glue/viewers/custom/qt/custom_viewer.py b/glue/viewers/custom/qt/custom_viewer.py index 10abf5605..9aae73bef 100644 --- a/glue/viewers/custom/qt/custom_viewer.py +++ b/glue/viewers/custom/qt/custom_viewer.py @@ -1377,13 +1377,12 @@ def _list_components(self): """ Determine which components to list. - This can be overridden by subclassing to limit which components are visible to the user. """ comps = list(set([c for l in self.container.layers - for c in l.data.components if not c._hidden])) + for c in (l.data.main_components + l.data.derived_components)])) comps = sorted(comps, key=lambda x: x.label) return comps diff --git a/glue/viewers/image/layer_artist.py b/glue/viewers/image/layer_artist.py index d2bc2b7b2..05de7dcd5 100644 --- a/glue/viewers/image/layer_artist.py +++ b/glue/viewers/image/layer_artist.py @@ -14,7 +14,7 @@ from glue.utils import color2rgb from glue.core.link_manager import is_equivalent_cid from glue.core import Data, HubListener -from glue.core.message import ComponentsChangedMessage +from glue.core.message import ComponentsChangedMessage, ExternallyDerivableComponentsChangedMessage from glue.external.modest_image import imshow @@ -36,6 +36,10 @@ def __init__(self, axes, viewer_state, layer_state=None, layer=None): handler=self._update_compatibility, filter=self._is_data_object) + self.layer.hub.subscribe(self, ExternallyDerivableComponentsChangedMessage, + handler=self._update_compatibility, + filter=self._is_data_object) + self._update_compatibility() def _is_data_object(self, message):