[#186925787]: Fix pairwise comparison #393
Conversation
slobodan-ilic
commented
Jan 27, 2024
slobodan-ilic
commented
- Pairwise comparison t-stats to use weighted bases
slobodan-ilic
force-pushed
the
fix-column-bases-186925787
branch
6 times, most recently
from
e483fcb to
d287a8b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #393 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 20
Lines 4620 4621 +1
=========================================
+ Hits 4620 4621 +1 ☔ View full report in Codecov by Sentry. |
src/cr/cube/matrix/measure.py
Outdated
| @@ -1545,21 +1545,27 @@ def _column_bases(self): | |||
| calculated column bases (either 'effective' or unweighted counts) for | |||
| the analysis. | |||
| """ | |||
| unweighted_blocks = self._second_order_measures.column_unweighted_bases.blocks | |||
| # unweighted_blocks = self._second_order_measures.column_unweighted_bases.blocks | |||
There was a problem hiding this comment.
get rid of commented code plz :D
src/cr/cube/matrix/measure.py
Outdated
| # unweighted_blocks[0][0] ** 2 / squared_blocks[0][0], | ||
| # unweighted_blocks[0][1] ** 2 / squared_blocks[0][1], |
src/cr/cube/matrix/assembler.py
Outdated
| else _SortRowsByMarginalHelper | ||
| if collation_method == CM.MARGINAL | ||
| else _RowOrderHelper | ||
| else ( |
There was a problem hiding this comment.
IMHO this is quite difficult to read. Could you think about something more readable?
There was a problem hiding this comment.
yeah this is crap... I dunno man... will try to figure out something better... maybe the switch?
There was a problem hiding this comment.
Is this better:
@classmethod
def factory(cls, cube, dimensions, slice_idx):
"""Return _BaseCubeMeans subclass instance appropriate to `cube`.
Raises `ValueError` if the cube-result does not include a cube-means measure.
"""
if cube.means is None:
raise ValueError("cube-result does not contain cube-means measure")
def get_cube_means_cls(dim_types):
if dim_types == (DT.MR, DT.MR):
return _MrXMrCubeMeans
if dim_types[0] == DT.MR:
return _MrXCatCubeMeans
if dim_types[1] == DT.MR:
return _CatXMrCubeMeans
return _CatXCatCubeMeans
dimension_types = cube.dimension_types[-2:]
CubeMeansCls = get_cube_means_cls(dimension_types)
return CubeMeansCls(
dimensions, cube.means[cls._slice_idx_expr(cube, slice_idx)]
)
There was a problem hiding this comment.
yes I like it more
src/cr/cube/matrix/measure.py
Outdated
| ], | ||
| ] | ||
| return effective_blocks | ||
| return unweighted_blocks | ||
| # return unweighted_blocks | ||
| return weighted_blocks |
There was a problem hiding this comment.
I think the outer return here is what happens in the absence of weighting, or absence of w^2. In the first case (no weights) weighted == unweighted, which would be fine. In the second instance, where you might have weights but fail to supply w^2, unweighted would still be correct here. The bug was only in the effective calculation above.
There was a problem hiding this comment.
@malecki Can you re-check this? I think I figured it out finally.
* Pairwise comparison t-stats to use weighted bases
slobodan-ilic
force-pushed
the
fix-column-bases-186925787
branch
from
a5f65ca to
f6f1dee
Compare
