[WIP] FIx smoothing and aggregation for unbalanced expected #466
Conversation
|
I am confused about the test, if I manually run the same command that it should be running it works for me, while pytest complains about exit code 2. |
| "n_pixels": _NUM_VALID, | ||
| "n_contacts": "count.sum", | ||
| "contact_freq": "count.avg", | ||
| "smooth_suffix": ".smoothed", |
There was a problem hiding this comment.
Consider renaming "suffix", "smooth suffix seems rather confusing and hard to generalize in future.
| @@ -1017,25 +1019,43 @@ def expected_cis( | |||
|
|
|||
| # additional smoothing and aggregating options would add columns only, not replace them | |||
| if smooth: | |||
| if clr_weight_name is None: | |||
| # result["count.avg"] = result["count.sum"] / result["n_valid"] | |||
There was a problem hiding this comment.
What is this commented out line?
| @@ -476,6 +476,54 @@ def test_expected_smooth_cli(request, tmpdir): | |||
| assert _delta_smooth_agg < 0.02 | |||
|
|
|||
|
|
|||
| def test_expected_smooth_nobalance_cli(request, tmpdir): | |||
| ) | ||
| # add smoothed columns to the result (only balanced for now) | ||
| result = result.merge( | ||
| result_smooth[["balanced.avg.smoothed", _DIST]], | ||
| result_smooth[[cols["contact_freq"] + cols["smooth_suffix"], _DIST]], |
There was a problem hiding this comment.
I find this notation very hard to read. Is it possible to retain something like this?
"count.sum" + suffix / without dictionary, having the regular notation
I endorse both readability and flexibility of the code, and here it looks like the balance is shifted towards flexibility.
| cols["contact_freq"] | ||
| + cols["smooth_suffix"]: cols["contact_freq"] | ||
| + cols["smooth_suffix"] | ||
| + ".agg" |
There was a problem hiding this comment.
".agg" is not in the cols dict anymore, although it's another type of suffix!
Consistency would be great.
No description provided.