Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-11038

Design DIAObject API

    Details

      Description

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

        Attachments

          Activity

          Hide
          cmorrison Chris Morrison added a comment -

          Created initial DIAObject class skeleton in lsst-dm/ap_association

          Show
          cmorrison Chris Morrison added a comment - Created initial DIAObject class skeleton in lsst-dm/ap_association
          Hide
          cmorrison Chris Morrison added a comment -
          Show
          cmorrison 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
          ebellm 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
          ebellm 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
          krughoff 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))
            

          Show
          krughoff 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
          cmorrison 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
          cmorrison 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
          ebellm 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
          ebellm 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
          cmorrison Chris Morrison added a comment -

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

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

            People

            • Assignee:
              cmorrison Chris Morrison
              Reporter:
              cmorrison Chris Morrison
              Reviewers:
              Eric Bellm
              Watchers:
              Chris Morrison, Eric Bellm, Simon Krughoff
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel