search

judithabk6 / med_bench

BSD 3-Clause "New" or "Revised" License
8 stars 3 forks source link

pytest issues #47

Closed houssamzenati closed 7 months ago

houssamzenati commented 7 months ago

message d'erreur suivant:

ERROR tests/estimation/test_get_estimation.py::test_total_is_direct_plus_indirect[multiply_robust_forest_calibration_cf-dict_param1] - TypeError: multiply_robust_efficient() got an unexpected keyword argument 'clip'

Remplacer clip dans le fichier src/get_estimation.py aux lignes 484, 498, 512, 526, 540, 554, 569, 583, 598, 612, 626, 640, 655, 669 par trim

l'argument supporté par la fonction multiply_robust_efficient est trim et non pas clip

bthirion commented 7 months ago

Indeed. Can you open a PR ?

judithabk6 commented 7 months ago

looking into details, it's more complicated than that. The clip argument in multiply_robust_efficient was renamed trim, but the trim argument is actually used to do clipping and not trimming, so this is misleading. So the fix should rather be to replace trim by clip in the multiply_robust_efficient implementation. (commit a2d8e02, was missed in the PR review)

I think this was done in some attempt to unify things. We should have a global decision on whether we want clip or trim or both or neither in the package, for all estimators.

bthirion commented 7 months ago

See #55 I propose to replace trim by clip wherever it is found. @judithabk6 is that OK ?