# Design DIAObject API

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
4
• Sprint:
Alert Production F17 - 7
• Team:

## Description

Come up with a skeleton design for DIAObjects in ap_association. Partly to be discuss during UW brownbag on 6/28/17

## Activity

Hide
Chris Morrison added a comment -

Created initial DIAObject class skeleton in lsst-dm/ap_association

Show
Chris Morrison added a comment - Created initial DIAObject class skeleton in lsst-dm/ap_association
Hide
Chris Morrison added a comment -
Show
Chris Morrison added a comment - Simon Krughoff and Eric Bellm take a look at https://github.com/lsst-dm/ap_association/blob/master/python/lsst/ap/association/dia_object.py and https://github.com/lsst-dm/ap_association/blob/master/tests/test_dia_object.py and let me know what you think of the initial direction for this dia_object.py class.
Hide
Eric Bellm added a comment -

Hi Chris,

Looks like a great start. A couple of naming comments:

• I don't think "initialized" is quite the word I'd use for the state when the summary stats are computed (and thus self-consistent with the source catalog). If nothing else it shares a connotation with the _init_ method, and we can have a perfectly useful DIAObject even with the summary stats not computed. I don't immediately have a great alternative. "summarized"? "stats_computed"?
• We may want a boolean argument to _init_ that allows us not to compute the summary stats, even if we don't pass an object_source_record.
• rather than "compute_light_curve", I'd say "retrieve_light_curve" (or maybe "return_")--I don't think there's any actual computation happening, it's just a lookup.
• do we need or want syntactic sugar for accessing specific summary stats? I'm not sure what SourceRecord gives us out of the box, but it might be nice to be able to do something like

 diaObj.stats.magRMS diaObj.stats['magRMS'] 

without retrieving the whole record

Show
Eric Bellm added a comment - Hi Chris, Looks like a great start. A couple of naming comments: I don't think "initialized" is quite the word I'd use for the state when the summary stats are computed (and thus self-consistent with the source catalog). If nothing else it shares a connotation with the _ init _ method, and we can have a perfectly useful DIAObject even with the summary stats not computed. I don't immediately have a great alternative. "summarized"? "stats_computed"? We may want a boolean argument to _ init _ that allows us not to compute the summary stats, even if we don't pass an object_source_record. rather than "compute_light_curve", I'd say "retrieve_light_curve" (or maybe "return_")--I don't think there's any actual computation happening, it's just a lookup. do we need or want syntactic sugar for accessing specific summary stats? I'm not sure what SourceRecord gives us out of the box, but it might be nice to be able to do something like diaObj.stats.magRMS diaObj.stats['magRMS'] without retrieving the whole record
Hide
Simon Krughoff added a comment -

• naming... initialized --> updated?
• I'm not as averse to saying compute_light_curve. We will store flux in the records and may want lightcurves in magnitudes. That requires a computation. I'm not hugely invested.
• I was thinking about using properties for this. It lets us have things that look like attributes on the DiaObject but makes it so they can't be set directly from code.

 class DiaObject(object):  @property  def magRMS(self):  return self.stats.get('magRMS') 

 class DiaObject(object):  for field in self.stats.schema:  name = field.name  getter = lambda x : x.stats.get(name)  self.__setattr__(name, property(getter, None)) 

Show
Simon Krughoff added a comment - Couple of comments naming... initialized --> updated ? I'm not as averse to saying compute_light_curve . We will store flux in the records and may want lightcurves in magnitudes. That requires a computation. I'm not hugely invested. I was thinking about using properties for this. It lets us have things that look like attributes on the DiaObject but makes it so they can't be set directly from code. class DiaObject( object ): @property def magRMS( self ): return self .stats.get( 'magRMS' ) or less readably class DiaObject( object ): for field in self .stats.schema: name = field.name getter = lambda x : x.stats.get(name) self .__setattr__(name, property (getter, None ))
Hide
Chris Morrison added a comment -

Created initial API for DIAObject class. The first pass can be found at: https://github.com/lsst-dm/ap_association/blob/master/python/lsst/ap/association/dia_object.py

Show
Chris Morrison added a comment - Created initial API for DIAObject class. The first pass can be found at: https://github.com/lsst-dm/ap_association/blob/master/python/lsst/ap/association/dia_object.py
Hide
Eric Bellm added a comment -

I'm happy with the structure, but there are a bunch of typos to fix.

49: DIA sources -> DIASources
53: “whenever a the”: delete “a”
54: “initialize method” -> “update method”
65: “statictics”
67: “statictis”, “nessisary”
72: -> DIASources (plural)
93: _upated
96: “for accessing <values? attributes?> stored
126: udated -> _updated
139 _upated
147: it’s -> its
148: DIAObjects should be DIASources
(several more _upated)…
160: udated

Show
Eric Bellm added a comment - I'm happy with the structure, but there are a bunch of typos to fix. 49: DIA sources -> DIASources 53: “whenever a the”: delete “a” 54: “initialize method” -> “update method” 65: “statictics” 67: “statictis”, “nessisary” 72: -> DIASources (plural) 93: _upated 96: “for accessing <values? attributes?> stored 126: udated -> _updated 139 _upated 147: it’s -> its 148: DIAObjects should be DIASources (several more _upated)… 160: udated
Hide
Chris Morrison added a comment -

Finished review comments and committed to lsst-dm/ap_association repository.

Show
Chris Morrison added a comment - Finished review comments and committed to lsst-dm/ap_association repository.

## People

• Assignee:
Chris Morrison
Reporter:
Chris Morrison
Reviewers:
Eric Bellm
Watchers:
Chris Morrison, Eric Bellm, Simon Krughoff