DM-47648 : Return table of unassociated objects #254
Conversation
Gerenjie
force-pushed
the
tickets/DM-47648
branch
from
263f939 to
2a22a13
Compare
Gerenjie
force-pushed
the
tickets/DM-47648
branch
from
4ff53f7 to
0b8fb1a
Compare
| solarSystemObjectTable, exposure) | ||
| associatedSsSources = ssoAssocResult.ssSourceData | ||
| return associatedSsSources | ||
| return self.solarSystemAssociator.run(sourceTable.to_pandas(), |
There was a problem hiding this comment.
I think I missed before that you are converting the source table to astropy, and then a few lines later converting it back to pandas. Can you find a different way to ensure that the ra and dec columns are added and in degrees?
There was a problem hiding this comment.
Changed the doc -- it starts out as a lsst.afw.table.SourceCatalog.
| @@ -144,31 +150,10 @@ def run(self, | |||
| """ | |||
There was a problem hiding this comment.
Please update the Returns section. There are now many additional components to the results struct.
| "residual_ras", "residual_decs"]) | ||
| associatedSsSources=Table(names=["ssObjectId", "ra", "dec", "obs_position", "obj_position", | ||
| "residual_ras", "residual_decs"]), | ||
| unassociatedSsObjects=Table(names=emptySolarSystemObjects.columns) |
There was a problem hiding this comment.
This is fine as it is, but are you expecting the column names to change?
| @@ -242,13 +249,14 @@ def _radec_to_xyz(self, ras, decs): | |||
|
|
|||
| return vectors | |||
|
|
|||
| def _return_empty(self, diaSourceCatalog): | |||
| def _return_empty(self, diaSourceCatalog, emptySolarSystemObjects): | |||
| @@ -163,8 +163,14 @@ def run(self, diaSourceCatalog, solarSystemObjects, exposure): | |||
| decs.append(dia_dec) | |||
| expected_ras.append(ssObject["ra"]) | |||
| expected_decs.append(ssObject["dec"]) | |||
|
|
|||
| self.log.info("Successfully associated %d SolarSystemObjects.", nFound) | |||
| associated.append(True) | |||
There was a problem hiding this comment.
associated is only used to set maskedObjects['associated'] below. Could you instead set the appropriate index of maskedObjects['associated'] directly to True or False here?
|
|
||
| self.log.info("Attempting to associate %d objects...", nSolarSystemObjects) |
There was a problem hiding this comment.
Why did you delete this log message?
There was a problem hiding this comment.
Amalgamated into "Successfully associated %d / %d SolarSystemObjects.", which I prefer since multithreaded log messages shuffle the lines together.
| @@ -125,12 +125,11 @@ def run(self, diaSourceCatalog, solarSystemObjects, exposure): | |||
| maskedObjects = self._maskToCcdRegion( | |||
| solarSystemObjects, | |||
| exposure, | |||
| solarSystemObjects["Err(arcsec)"].max()) | |||
| solarSystemObjects["Err(arcsec)"].max()).copy() | |||
There was a problem hiding this comment.
I don't think you want this .copy() statement. It is making a copy of the return value of self._maskToCcdRegion, which does not seem useful.
There was a problem hiding this comment.
self._maskToCcdRegion returns a copy-by-reference mask of solarSystemObjects, which I can't set a column of (i.e. associated later). Copying the dataframe makes it alterable.
|
I forgot to mention: please edit the "merge" commit to have a more descriptive message. |
Gerenjie
force-pushed
the
tickets/DM-47648
branch
3 times, most recently
from
4966bc5 to
7c860aa
Compare
Gerenjie
force-pushed
the
tickets/DM-47648
branch
from
7c860aa to
866eb43
Compare
Gerenjie
force-pushed
the
tickets/DM-47648
branch
from
866eb43 to
2c47c4e
Compare
Adds a new dataset, unAssocSsObjects, to the struct returned by ssoAssociation and sends it to the Butler as
ssSingleFrameUnassociatedObjectsthrough ssSingleFrameAssociation. Doesn't do anything to diaPipe, so this only affects SingleFrame.