Follow-up items from the review of #579
Test coverage gaps
-
test_combine_stack only asserts structure (isinstance, operator) but not numerical correctness of update() output. Should add an np.testing.assert_allclose check similar to test_combine_stack_preserves_all_series.
-
test_nested_combine_take only asserts that types and operators are preserved after take(). Should add a numerical correctness assertion to verify the subset transform produces the same update() output as a freshly-fitted transform on the subset.
Minor efficiency issue
Combine.stack() does copy.deepcopy(transforms[0]) and then immediately overwrites out.tfm1 and out.tfm2, so the deepcopy of those two attributes is wasted work. This mirrors the pattern in _BaseLagTransform.stack() so it's consistent, but could be cleaned up if performance becomes a concern.
Follow-up items from the review of #579
Test coverage gaps
test_combine_stackonly asserts structure (isinstance,operator) but not numerical correctness ofupdate()output. Should add annp.testing.assert_allclosecheck similar totest_combine_stack_preserves_all_series.test_nested_combine_takeonly asserts that types and operators are preserved aftertake(). Should add a numerical correctness assertion to verify the subset transform produces the sameupdate()output as a freshly-fitted transform on the subset.Minor efficiency issue
Combine.stack()doescopy.deepcopy(transforms[0])and then immediately overwritesout.tfm1andout.tfm2, so the deepcopy of those two attributes is wasted work. This mirrors the pattern in_BaseLagTransform.stack()so it's consistent, but could be cleaned up if performance becomes a concern.