Review changes

python/lsst/ap/association/filterDiaSourceCatalog.py

@@ -105,14 +105,14 @@ class FilterDiaSourceCatalogConfig(
     )
 
     doTrailedSourceFilter = pexConfig.Field(
-        doc="Run trailedSourceFilter to remove long trailed sources from "
-            "output catalog.",
+        doc="Run trailedSourceFilter to remove long trailed sources from the"
+            "diaSource output catalog.",
         dtype=bool,
         default=True,
     )
 
     doWriteTrailedSources = pexConfig.Field(
-        doc="Write trailed sources to a table.",
+        doc="Write trailed diaSources sources to a table.",
         dtype=bool,
         default=True,
         deprecated="Trailed sources will not be written out during production."
@@ -144,7 +144,7 @@ def run(self, diaSourceCat, diffImVisitInfo):
         diaSourceCat : `lsst.afw.table.SourceCatalog`
             Catalog of sources measured on the difference image.
         diffImVisitInfo:  `lsst.afw.image.VisitInfo`
-            Visit information for a particular exposure.
+            VisitInfo for the difference image corresponding to diaSourceCat.
 
         Returns
         -------
@@ -156,7 +156,7 @@ def run(self, diaSourceCat, diffImVisitInfo):
                 The catalog of rejected sky sources.
             ``longTrailedDiaSources`` : `astropy.table.Table`
                 DiaSources which have trail lengths greater than
-                max_trail_length/second*exposure_time.
+                max_trail_length*exposure_time.
         """
         rejectedSkySources = None
         exposure_time = diffImVisitInfo.exposureTime
@@ -174,9 +174,9 @@ def run(self, diaSourceCat, diffImVisitInfo):
             longTrailedDiaSources = diaSourceCat[trail_mask].copy(deep=True)
             diaSourceCat = diaSourceCat[~trail_mask]
 
-            self.log.info(
-                "%i DiaSources exceed max_trail_length, dropping from source "
-                "catalog." % len(diaSourceCat))
+            self.log.info("%i DiaSources exceed max_trail_length %f arcseconds per second, "
+                          "dropping from source catalog."
+                          % (self.config.max_trail_length, len(diaSourceCat)))
             self.metadata.add("num_filtered", len(longTrailedDiaSources))
 
             if self.config.doWriteTrailedSources:
@@ -198,8 +198,9 @@ def _check_dia_source_trail(self, dia_sources, exposure_time):
         end points.
 
         Return a mask of sources with lengths greater than
-        ``config.max_trail_length``  multiplied by the exposure time in
-        seconds. Additionally, set mask if
+        (``config.max_trail_length`` multiplied by the exposure time)
+        arcseconds.
+        Additionally, set mask if
         ``ext_trailedSources_Naive_flag_off_image`` is set or if
         ``ext_trailedSources_Naive_flag_suspect_long_trail`` and
         ``ext_trailedSources_Naive_flag_edge`` are both set.

tests/test_filterDiaSourceCatalog.py

@@ -44,30 +44,42 @@ def setUp(self):
             dataset.addSource(10000.0, geom.Point2D(srcIdx, self.yLoc))
         schema = dataset.makeMinimalSchema()
         schema.addField("sky_source", type="Flag", doc="Sky objects.")
-        schema.addField('ext_trailedSources_Naive_flag_off_image', type="Flag", doc="off_image")
+        schema.addField('ext_trailedSources_Naive_flag_off_image', type="Flag",
+                        doc="Trailed source which extends off image")
         schema.addField('ext_trailedSources_Naive_flag_suspect_long_trail',
-                        type="Flag", doc="suspect_long_trail")
-        schema.addField('ext_trailedSources_Naive_flag_edge', type="Flag", doc="edge")
-        schema.addField('ext_trailedSources_Naive_flag_nan', type="Flag", doc="nan")
-        schema.addField('ext_trailedSources_Naive_length', type="F", doc="trail length")
+                        type="Flag", doc="Trailed source with a trail length greater "
+                                         "than three times the psf.")
+        schema.addField('ext_trailedSources_Naive_flag_edge', type="Flag",
+                        doc="Trailed source has one or more end points falling on an edge pixel.")
+        schema.addField('ext_trailedSources_Naive_flag_nan', type="Flag",
+                        doc="Trailed source has a nan for one or more end points.")
+        schema.addField('ext_trailedSources_Naive_length', type="F",
+                        doc="Length of the source trail.")
         _, self.diaSourceCat = dataset.realize(10.0, schema, randomSeed=1234)
         self.diaSourceCat[0:self.nSkySources]["sky_source"] = True
-        self.nLongTrailedSources = 0
+        # The last 10 sources will all contained trail length measurements,
+        # increasing in size by 1.5 arcseconds. Only the last three will have
+        # lengths which are too long and will be filtered out.
+        self.nFilteredTrailedSources = 0
         for srcIdx in range(5, 15):
             self.diaSourceCat[srcIdx]["ext_trailedSources_Naive_length"] = 1.5*(srcIdx-4)
             if 1.5*(srcIdx-4) > 36000/3600.0/24.0 * 30.0:
-                self.nLongTrailedSources += 1
+                self.nFilteredTrailedSources += 1
+        # Setting a combination of flags for filtering in tests
         self.diaSourceCat[5]["ext_trailedSources_Naive_flag_off_image"] = True
         self.diaSourceCat[6]["ext_trailedSources_Naive_flag_suspect_long_trail"] = True
-        self.nLongTrailedSources += 2
         self.diaSourceCat[6]["ext_trailedSources_Naive_flag_edge"] = True
+        # As only two of these flags are set, the total number of filtered
+        # sources will be self.nFilteredTrailedSources + 2
+        self.nFilteredTrailedSources += 2
         self.config = FilterDiaSourceCatalogConfig()
         mjd = 57071.0
         self.utc_jd = mjd + 2_400_000.5 - 35.0 / (24.0 * 60.0 * 60.0)
 
         self.visitInfo = afwImage.VisitInfo(
-            # Incomplete VisitInfo; Python constructor allows any value to
-            # be defaulted.
+            # This incomplete visitInfo is sufficient for testing because the
+            # Python constructor sets all other required values to some
+            # default.
             exposureTime=30.0,
             darkTime=3.0,
             date=dafBase.DateTime(mjd, system=dafBase.DateTime.MJD),
@@ -105,18 +117,18 @@ def test_run_with_filter_sky_only(self):
         self.assertEqual(len(self.diaSourceCat), self.nSources)
 
     def test_run_with_filter_trailed_sources_only(self):
-        """Test that when only the trail filter is turned on the the last three
-        sources which have long trails, the source with both the suspect trail
-        and edge trail flag are set, and the source which is flagged as
-        continuing off edge are all filtered out. All sky objects should remain
-        in the catalog.
+        """Test that when only the trail filter is turned on the correct number
+        of sources are filtered out. The filtered sources should be the last
+        three sources which have long trails, one source where both the suspect
+        trail and edge trail flag are set, and one source where off_image is
+        set. All sky objects should remain in the catalog.
         """
         self.config.doRemoveSkySources = False
         self.config.doWriteRejectedSkySources = False
         self.config.doTrailedSourceFilter = True
         filterDiaSourceCatalogTask = FilterDiaSourceCatalogTask(config=self.config)
         result = filterDiaSourceCatalogTask.run(self.diaSourceCat, self.visitInfo)
-        nExpectedFilteredSources = self.nSources - self.nLongTrailedSources
+        nExpectedFilteredSources = self.nSources - self.nFilteredTrailedSources
         self.assertEqual(len(result.filteredDiaSourceCat), nExpectedFilteredSources)
         self.assertEqual(len(self.diaSourceCat), self.nSources)
 
@@ -129,7 +141,7 @@ def test_run_with_all_filters(self):
         self.config.doTrailedSourceFilter = True
         filterDiaSourceCatalogTask = FilterDiaSourceCatalogTask(config=self.config)
         result = filterDiaSourceCatalogTask.run(self.diaSourceCat, self.visitInfo)
-        nExpectedFilteredSources = self.nSources - self.nSkySources - self.nLongTrailedSources
+        nExpectedFilteredSources = self.nSources - self.nSkySources - self.nFilteredTrailedSources
         # 5 filtered out sky sources
         # 4 filtered out trailed sources, 2 with long trails 2 with flags
         # 6 sources left