Skip to content

Commit

Permalink
Added responses to first review comments.
Browse files Browse the repository at this point in the history 
Missed some edits in last commit. Now added.

Implimented second round of reviewer comments

Changed to dictionary of expected values in unittest

Finished second round of reviewer comments.
  • Loading branch information
@morriscb
morriscb committed Aug 1, 2017
1 parent 48ba7b8 commit 2c21a0f
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 105 deletions.
98 changes: 33 additions & 65 deletions python/lsst/ap/association/dia_object.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import lsst.afw.table as afwTable
import lsst.afw.geom as afwGeom
from lsst.afw.coord import averageCoord

__all__ = ["DIAObject", "make_minimal_dia_object_schema"]

Expand All @@ -39,13 +40,13 @@ def make_minimal_dia_object_schema():
"""

schema = afwTable.SourceTable.makeMinimalSchema()
# For the MVP/S we currently only care about the position though
# in the future we will add summary computations for fluxes etc.
# as well as their errors.

# For the MVP/S we currently only care about the scatter on the possition
# so we add those.
schema.addField("coord_ra_rms", doc="rms position in ra/dec",
type="Angle")
schema.addField("coord_dec_rms", doc="rms position in ra/dec",
type="Angle")
# In the future we would like to store a covariance of the coordinate.
# This functionality is not defined in currenting in the stack, so we will
# hold off until it is implemented. This is to be addressed in DM-7101.

return schema

Expand All @@ -54,27 +55,12 @@ class DIAObject(object):
""" A class specifying a collection of single frame difference image
sources and statistics on these collections.
Attributes
----------
_dia_object_record : lsst.afw.table.SourceRecord
A SourceRecord object containing the summary statistics for the
collection of DIASources this DIAObject represents (e.g. median
RA/DEC position).
_dia_source_catalog : lsst.afw.table.SourceCatalog
A set of SourceRecords specifying the DIASources that make up
this DIAObject.
_updated : bool
boolean specifying if the summary statistics for this DIAObject have
been updated with the current set of DIASources in the SourceCatalog.
This variable should be set to false whenever the SourceCatalog
of DIASources changes and set to true when the initialize method is
run.
"""
def __init__(self, dia_source_catalog, object_source_record=None):
""" Create a DIAObject given an input SourceCatalog of
DIASources.
Takes as input an lsst.afw.table.SourceCaatlog object specifying a
Takes as input an lsst.afw.table.SourceCatlog object specifying a
collection of DIASources that make up this DIAObject. The optional
input object_source_record should contain summary statistics on the
SourceCatalog of DIASources. Using this optional input escapes the
Expand Down Expand Up @@ -110,16 +96,15 @@ def __init__(self, dia_source_catalog, object_source_record=None):
self._updated = True

def get(self, name):
""" Return the data stored in column name within the internal
dia_object_record.
""" Retrieve a specific summary statistic from this DIAObject
Parameters
----------
name : str or lsst.afw.table.Key
Return
------
An lsst.afw data type
A SourceRecord column value
"""

# This will in the future be replaced with a overwritting of __getattr
Expand All @@ -132,11 +117,7 @@ def update(self):
Store these summaries (e.g. median RA/DEC position, fluxes...) in
the object_source_record attribute and set the class variable
updated to True
Returns
-------
None
updated to True.
"""

self._updated = False
Expand All @@ -145,45 +126,34 @@ def update(self):
# is currently contious and if not we make a deep copy.
if not self._dia_source_catalog.isContiguous():
tmp_dia_source_catalog = self._dia_source_catalog.copy(deep=True)
del self._dia_source_catalog
self._dia_source_catalog = tmp_dia_source_catalog

self._compute_summary_statistics()

self._updated = True

return None

def _compute_summary_statistics(self):
""" Retrive properties from DIASourceCatalog attribute and update the
""" Retrieve properties from DIASourceCatalog attribute and update the
summary statistics that represent this DIAObject
Returns
-------
None
"""

# Loop through DIASources, compute summary statistics (TBD) and store
# them in dia_object_record attribute.

for name in self.schema.getNames():
# For the MVP/S we are only dealing with angles so we skip over
# everything that isn't coord_ra or coord_dec.
if name != 'coord_ra' and name != 'coord_dec':
continue

mean_value = np.mean(self._dia_source_catalog[name])
# Currently hard coded to work with angles.
self._dia_object_record.set(name, afwGeom.Angle(mean_value))
# If we only have 1 source we can't compute an rms and hence we
# set the the rms variables to NaN.
if self.n_dia_sources > 1:
self._dia_object_record[name + '_rms'] = afwGeom.Angle(
np.std(self._dia_source_catalog[name]))
else:
self._dia_object_record[name + '_rms'] = afwGeom.Angle(np.nan)

return None
self._compute_mean_coordinate()

# In the future we will calculate covariances on this centroid,
# however generalized coordinate covariances are not defined (DM-7101)
# we also do not need them yet for the MVP/S

def _compute_mean_coordinate(self):
""" Compute the mean coordinate of this DIAObject given the current
DIASources associated with it.
"""

coord_list = [src.getCoord() for src in self._dia_source_catalog]
ave_coord = averageCoord(coord_list)
self._dia_object_record.setCoord(ave_coord)

def append_dia_source(self, input_dia_source_record):
""" Append the input_dia_source to the dia_source_catalog attribute.
Expand All @@ -195,23 +165,19 @@ def append_dia_source(self, input_dia_source_record):
input_dia_source : lsst.afw.table.SourceRecord
Single DIASource object to append to this DIAObject's source
catalog.
Return
------
None
"""

# Since we are adding to the SourceCatalog our summary statistics are
# no longer valid. We set this to false and hold off on recomputing
# them until we are finished adding sources.
self._updated = False

self._dia_source_catalog.append(input_dia_source_record)

return None
self._dia_source_catalog.append(
self._dia_source_catalog.getTable().copyRecord(
input_dia_source_record))

def get_light_curve(self):
""" Retreve the light curve of fluxes for the DIASources that make up
""" Retrieve the light curve of fluxes for the DIASources that make up
this DIAObject.
Returns
Expand All @@ -223,7 +189,9 @@ def get_light_curve(self):
# Right now I'm making this the same as returning the
# dia_source_catalog.

return self.dia_source_catalog()
raise NotImplimentedError(
"Light curves not yet implimented. Use dia_source_catalog property"
"instead.")

@property
def is_updated(self):
Expand Down
71 changes: 31 additions & 40 deletions tests/test_dia_object.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,6 @@ def create_test_dia_sources(n_sources=5):

class TestDIAObject(unittest.TestCase):

def setUp(self):
""" Create the DIAObjct schema we will use throughout these tests.
"""
self.dia_obj_schema = make_minimal_dia_object_schema()

def tearDown(self):
""" Delete the schema when we are done.
"""
del self.dia_obj_schema

def test_init(self):
""" Test DIAObject creation and if we can properly instantiate a
DIAObject from an already create dia_object_record.
Expand All @@ -76,11 +66,17 @@ def test_init(self):
dia_obj = DIAObject(single_source, None)
dia_obj_dup = DIAObject(single_source, dia_obj.dia_object_record)

self._compare_dia_object_values(dia_obj, 0, 0.0, np.nan)
self._compare_dia_object_values(dia_obj_dup, dia_obj.id, 0.0, np.nan)
compare_values = {
"coord_ra": 0.0,
"coord_dec": 0.0
}

self._compare_dia_object_values(dia_obj, 0, compare_values)
self._compare_dia_object_values(dia_obj_dup, dia_obj.id,
compare_values)

def _compare_dia_object_values(self, dia_object, expected_id,
expected_mean, expected_std):
expected_dict):
""" Compare values computed in the compute_summary_statistics
DIAObject class method with those expected.
Expand All @@ -90,33 +86,18 @@ def _compare_dia_object_values(self, dia_object, expected_id,
Input DIAObect to test
expected_id : int
Expected id of the DIAObject
expected_mean : float
Expected mean value of the parameters to be tested.
expected_std : float
Expected standard diviation of the DIAObject.
Return
------
None
expected_dict : dict
Dictionary of field name and expected value
"""

for name in self.dia_obj_schema.getNames():
if name == 'parent':
continue
if name == 'id':
self.assertEqual(dia_object.get(name), expected_id)
continue

if name[-3:] != 'rms':
self.assertAlmostEqual(dia_object.get(name).asDegrees(),
expected_mean)
elif name[-3:] == 'rms' and np.isfinite(expected_std):
self.assertAlmostEqual(dia_object.get(name).asDegrees(),
expected_std)
elif name[-3:] == 'rms' and not np.isfinite(expected_std):
self.assertFalse(np.isfinite(dia_object.get(name).asDegrees()))

return None
self.assertEqual(dia_object.get('id'), expected_id)
for key in expected_dict.keys():
if isinstance(dia_object.get(key), afwGeom.Angle):
self.assertAlmostEqual(dia_object.get(key).asDegrees(),
expected_dict[key])
else:
self.assertAlmostEqual(dia_object.get(key),
expected_dict[key])

def test_update(self):
""" If we instantiate the class with a set of DIASources we test to
Expand All @@ -126,7 +107,12 @@ def test_update(self):
sources = create_test_dia_sources(5)
dia_obj = DIAObject(sources, None)

self._compare_dia_object_values(dia_obj, 0, 2.0, 1.4142135623730951)
compare_values = {
"coord_ra": 1.9987807133764057,
"coord_dec": 2.000608742419802
}

self._compare_dia_object_values(dia_obj, 0, compare_values)

def test_dia_source_append_and_update(self):
""" Test the appending of a DIASource to a DIAObject. We also
Expand All @@ -149,8 +135,13 @@ def test_dia_source_append_and_update(self):
self.assertEqual(associated_sources[-1].getCoord(),
sources[-1].getCoord())

compare_values = {
"coord_ra": 0.4999619199226218,
"coord_dec": 0.5000190382261059
}

dia_obj.update()
self._compare_dia_object_values(dia_obj, 0, 0.5, 0.5)
self._compare_dia_object_values(dia_obj, 0, compare_values)

def test_compute_light_curve(self):
""" Not implemented yet.
Expand Down

0 comments on commit 2c21a0f

Please sign in to comment.