DM-36058: Fix untested Pandas deprecation warnings in ap_association #162
Conversation
This commit removes some calls to Dataframe.append that were missed in 3fa22fb. The calls could not be detected because they were mocked out. I don't see a way to prevent such issues short of rewriting the tests to use real tables in place of mocks, but I don't know how to do so while following the correct table schema (which is subject to change).
| @@ -383,8 +383,9 @@ def run(self, | |||
| inplace=True) | |||
|
|
|||
| # Append new DiaObjects and DiaSources to their previous history. | |||
| diaObjects = loaderResult.diaObjects.append( | |||
| createResults.newDiaObjects.set_index("diaObjectId", drop=False), | |||
| diaObjects = pd.concat( | |||
There was a problem hiding this comment.
I'm very worried about using concat in our code like this: it can change the schema out from under us. Should we be using convert_dtypes here (and anywhere we use concat)? Could we move this away from pandas entirely and use numpy.concatenate or numpy.vstack?
If we had a way to get pd.concat to raise instead of silently casting to float, that might be more reasonable, but I don't think there is.
There was a problem hiding this comment.
I don't know anything about convert_dtypes, I just followed the instructions for how to phase out append. As for using pandas in general, see my comment on #160.
|
See discussion on #160 : I've just found that the diaForcedSource That's for a different ticket (not filed yet), so I think you're good to merge here. |
This PR removes references to
Dataset.appendthat weren't covered by unit tests, and tweaks the tests so that they don't make assumptions about the call signatures in application code. It does not try to expand test coverage to perform actual Pandas operations, since I don't see a way to introduce mock tables without making them brittle to external changes.