Two level pairwise t #204
Merged
Two level pairwise t #204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| @lazyproperty | ||
| def _df(self): | ||
| # if the cube to which the slice belongs is a CATxMRxITSELF |
There was a problem hiding this comment.
If it's possible i would add a docstring for degree of freedom just summarising the existing comment here
There was a problem hiding this comment.
I didn't write this class, I just reordered its methods while I was cleaning up the module, so I don't know what this method is supposed to do (and don't really want to right at the moment :).
ernestoarbitrio
requested changes
Jun 10, 2020
|
k, fixed the two docstrings, explanations on the rest :) |
Basic manipulation of transforms is general to all cube-partition sub-types. Move ownership of `._transforms_arg` instance variable to CubePartition in preparation for moving other shared transforms responsibilities.
Generalize `._transforms_dict` property to CubePartition.
Replace redundant local implementations with property on base class.
Also generally clean up while in the neighborhood. * Old-style classes have unexpected behavior differences in corner cases, in particular with mocks in unit tests. Make these new-style classes by adding (object) superclass. * Normalize method ordering. * Improve docstrings and formatting.
Make PairwiseSignificance.pairwise_indices() a classmethod call rather than requiring caller to instantiate a PairwiseSignificance object and then make the call.
* Add Python-literal expection facility * Condense same-form _Slice.pairwise_indices integration tests into a single parametrized test. * Add or rename a fixture or two as required.
Also add a couple omitted cases while we're in there and it's so easy.
Avoid using numpy types as interface (return) types. Use a tuple for the container of the column-idx tuples instead of an np.ndarray. Using a Python-native type makes using the method easier, requiring no special methods the user may not be as familiar with. Using an immutable type in a lazyproperty is wise because it prevents a caller from mutating a value that may be returned to a different caller in another call.
scanny
force-pushed
the
two-level-pairwise-t
branch
from
8b490fe
to
aa0a98f
Compare
ernestoarbitrio
approved these changes
Jun 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes required for two-level pairwise-t display, primarily adding
_Slice.pairwise_indices_altand_Slice.scale_mean_pairwise_indices_altalong with processing the transforms-dict construction parameter for multiple threshold values.