DM-28555: Add verbosity to ApPipe and DiaPipe DB errors #106
Conversation
| # if len(diaObjects) > 0: | ||
| # dups = diaObjects.iloc[[0, -1]] | ||
| # diaObjects = diaObjects.append(dups, sort=True) |
There was a problem hiding this comment.
Delete this commented-out code.
There was a problem hiding this comment.
Thanks for the catch. Was keeping this in until the decision was made on warn vs raise for finding duplicates. Will take it out.
| @@ -97,11 +98,19 @@ def run(self, exposure, apdb): | |||
| ``diaObjectId``, ``filterName``, ``diaSourceId`` columns. | |||
| (`pandas.DataFrame`) | |||
| """ | |||
| visit_info = exposure.getInfo().getVisitInfo() | |||
There was a problem hiding this comment.
Shouldn't this be visitInfo?
| "Duplicate DiaObjects created after association. This may " | ||
| "cause downstream pipeline issues. Dropping duplicated rows.") | ||
| # Drop duplicates via index and keep the first appearance. | ||
| diaObjects = diaObjects.groupby(diaObjects.index).first() |
There was a problem hiding this comment.
Here, and throughout these changes where you use groupby: does this change the ordering of the diaObjects?
There was a problem hiding this comment.
It might, but since the dataFrames are indexed on the object/source dataIds and continue to be so after this interaction, it won't matter. I rarely use iloc and only do so after matching it to the proper location within the arrays.
| "Duplicate DiaSources found after association and merging " | ||
| "with history. This is likely due to re-running data with an " | ||
| "already populated Apdb. If this was not the case then there " | ||
| "was a failure in Association which should not happen. " |
There was a problem hiding this comment.
How about "an unexpected failure in Association, and should be reported". From personal experience, I find it frustrating to get an error message that tells me that it shouldn't happen, without also telling me how to try to debug or fix it.
There was a problem hiding this comment.
I expanded a bit but left the heart of what you suggested.
Additinally add code to drop duplicates for now. Debug loadDiaCatalogs dup tests. Properly re-index multiIndex dataframes to not break downstream processing. Copy debugged duplicate detction code to all points. Commit after initial debug of must has_duplicates tests. Respond to reviewer. Fix variable name. Remove commented code.
Implement tests for inputing dups in association. Debug RuntimeError test. Fix numpy type warnings. Fix more numpy deprication warnings.
morriscb
force-pushed
the
tickets/DM-28555
branch
from
db57dc3
to
0fa287e
Compare
No description provided.