Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly deal with reassignment of Data.coords #1196

Merged
merged 4 commits into from
May 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ v0.13.3 (unreleased)
* Fixed a bug related to callback functions when restoring sessions.
[#1695]

* Fixed a bug that meant that setting Data.coords after adding
components didn't work as expected. [#1196]

* Fixed a Qt-related segmentation fault that occurred during the
testing process and may also affect users. [#1703]

Expand Down
27 changes: 25 additions & 2 deletions glue/core/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ def __init__(self, label="", coords=None, **kwargs):

Extra array-like keywords are extracted into components
"""
# Coordinate conversion object
self.coords = coords or Coordinates()

self._shape = ()

# Components
Expand All @@ -83,6 +82,9 @@ def __init__(self, label="", coords=None, **kwargs):
self._pixel_component_ids = ComponentIDList()
self._world_component_ids = ComponentIDList()

# Coordinate conversion object
self.coords = coords or Coordinates()

self.id = ComponentIDDict(self)

# Metadata
Expand Down Expand Up @@ -113,6 +115,20 @@ def __init__(self, label="", coords=None, **kwargs):
# uniquely identify them.
self.uuid = str(uuid.uuid4())

@property
def coords(self):
"""
The coordinates object for the data.
"""
return self._coords

@coords.setter
def coords(self, value):
if (hasattr(self, '_coords') and self._coords != value) or not hasattr(self, '_coords'):
self._coords = value
if len(self.components) > 0:
self._update_world_components(self.ndim)

@property
def subsets(self):
"""
Expand Down Expand Up @@ -548,14 +564,21 @@ def add_component_link(self, link, label=None):
return dc

def _create_pixel_and_world_components(self, ndim):
self._update_pixel_components(ndim)
self._update_world_components(ndim)

def _update_pixel_components(self, ndim):
for i in range(ndim):
comp = CoordinateComponent(self, i)
label = pixel_label(i, ndim)
cid = PixelComponentID(i, "Pixel Axis %s" % label, parent=self)
self.add_component(comp, cid)
self._pixel_component_ids.append(cid)

def _update_world_components(self, ndim):
for cid in self._world_component_ids[:]:
self.remove_component(cid)
self._world_component_ids.remove(cid)
if self.coords:
for i in range(ndim):
comp = CoordinateComponent(self, i, world=True)
Expand Down
52 changes: 50 additions & 2 deletions glue/core/tests/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import pytest
import numpy as np
from numpy.testing import assert_equal
from mock import MagicMock

from glue.external import six
Expand All @@ -14,6 +15,8 @@
from ..component_link import ComponentLink, CoordinateComponentLink, BinaryComponentLink
from ..coordinates import Coordinates
from ..data import Data, pixel_label
from ..link_helpers import LinkSame
from ..data_collection import DataCollection
from ..exceptions import IncompatibleAttribute
from ..hub import Hub
from ..registry import Registry
Expand Down Expand Up @@ -603,8 +606,6 @@ def test_foreign_pixel_components_not_in_visible():
"""Pixel components from other data should not be visible"""

# currently, this is trivially satisfied since all coordinates are hidden
from ..link_helpers import LinkSame
from ..data_collection import DataCollection

d1 = Data(x=[1], y=[2])
d2 = Data(w=[3], v=[4])
Expand Down Expand Up @@ -785,3 +786,50 @@ class CustomObject(object):
assert data2.meta['a'] == 1
assert data2.meta['b'] == 'test'
assert 'c' not in data2.meta


def test_update_coords():

# Make sure that when overriding coords, the world coordinate components
# are updated.

data1 = Data(x=[1, 2, 3])

assert len(data1.components) == 3

assert_equal(data1[data1.world_component_ids[0]], [0, 1, 2])

data2 = Data(x=[1, 2, 3])

assert len(data1.links) == 2
assert len(data2.links) == 2

data_collection = DataCollection([data1, data2])

assert len(data_collection.links) == 4

data_collection.add_link(LinkSame(data1.world_component_ids[0], data2.world_component_ids[0]))

assert len(data_collection.links) == 5

class CustomCoordinates(Coordinates):

def axis_label(self, axis):
return 'Custom {0}'.format(axis)

def world2pixel(self, *world):
return tuple([0.4 * w for w in world])

def pixel2world(self, *pixel):
return tuple([2.5 * p for p in pixel])

data1.coords = CustomCoordinates()

assert len(data1.components) == 3

assert_equal(data1[data1.world_component_ids[0]], [0, 2.5, 5])

assert sorted(cid.label for cid in data1.world_component_ids) == ['Custom 0']

# The link between the two world coordinates should be remove
assert len(data_collection.links) == 4
1 change: 1 addition & 0 deletions glue/viewers/matplotlib/qt/tests/test_data_viewer.py
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ def test_numerical_data_changed(self):
self.viewer.add_data(self.data)
assert self.draw_count == 1
data = Data(label=self.data.label)
data.coords = self.data.coords
for cid in self.data.visible_components:
data.add_component(self.data[cid] * 2, cid.label)
self.data.update_values_from_data(data)
Expand Down