Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Spike overlaps #256

Merged
merged 12 commits into from
Mar 24, 2021
Merged

Spike overlaps #256

merged 12 commits into from
Mar 24, 2021

Conversation

slobodan-ilic
Copy link
Contributor

No description provided.

@slobodan-ilic slobodan-ilic mentioned this pull request Mar 15, 2021
Closed
@coveralls
Copy link

coveralls commented Mar 15, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling bd8f4f1 on new-overlaps-spike into 9e2f56d on master.

ernestoarbitrio
ernestoarbitrio requested changes Mar 16, 2021
View reviewed changes
Copy link
Contributor

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments

src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/stripe/cubemeasure.py Outdated Show resolved Hide resolved
src/cr/cube/stripe/cubemeasure.py Outdated Show resolved Hide resolved
@slobodan-ilic slobodan-ilic force-pushed the new-overlaps-spike branch 2 times, most recently from c327df4 to 827a0b7 Compare March 18, 2021 17:44
ernestoarbitrio
ernestoarbitrio requested changes Mar 19, 2021
View reviewed changes
Copy link
Contributor

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per comments

src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/enums.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/assembler.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/assembler.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
scanny
scanny suggested changes Mar 20, 2021
View reviewed changes
Copy link
Contributor

@scanny scanny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll need to take another look once you addressed these and gotten docstrings written, but this should be a good start.

src/cr/cube/cubepart.py Outdated Show resolved Hide resolved
src/cr/cube/cubepart.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/assembler.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/assembler.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/cubemeasure.py Outdated Show resolved Hide resolved
src/cr/cube/stripe/measure.py Outdated Show resolved Hide resolved
src/cr/cube/stripe/measure.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
src/cr/cube/cube.py Outdated Show resolved Hide resolved
@slobodan-ilic slobodan-ilic force-pushed the new-overlaps-spike branch 2 times, most recently from f308c03 to 53a9911 Compare March 22, 2021 14:26
ernestoarbitrio
ernestoarbitrio requested changes Mar 22, 2021
View reviewed changes
Copy link
Contributor

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slobodan-ilic per comments. I just stop at the measure level cause i need to understand if something can be refactored without having t-stats and p-vals as separate measures. I'll take another look whenever you ready

src/cr/cube/matrix/cubemeasure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/assembler.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
@slobodan-ilic slobodan-ilic force-pushed the new-overlaps-spike branch 6 times, most recently from dfa0703 to b52419f Compare March 23, 2021 14:59
ernestoarbitrio
ernestoarbitrio requested changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per comments ... hope this will be the last iteration ... :D

src/cr/cube/matrix/cubemeasure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/cubemeasure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
src/cr/cube/matrix/measure.py Outdated Show resolved Hide resolved
tests/integration/test_pairwise_significance.py Outdated Show resolved Hide resolved
tests/integration/test_pairwise_significance.py Outdated Show resolved Hide resolved
tests/integration/test_pairwise_significance.py Outdated Show resolved Hide resolved
tests/integration/test_pairwise_significance.py Outdated Show resolved Hide resolved
tests/integration/test_pairwise_significance.py Outdated Show resolved Hide resolved
@slobodan-ilic slobodan-ilic force-pushed the new-overlaps-spike branch 3 times, most recently from 9767c04 to 07fa1ca Compare March 23, 2021 17:52
ernestoarbitrio
ernestoarbitrio approved these changes Mar 24, 2021
View reviewed changes
Copy link
Contributor

@ernestoarbitrio ernestoarbitrio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slobodan-ilic slobodan-ilic merged commit 89ffe34 into master Mar 24, 2021
@ernestoarbitrio ernestoarbitrio deleted the new-overlaps-spike branch December 20, 2022 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scanny scanny scanny requested changes

@ernestoarbitrio ernestoarbitrio ernestoarbitrio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@slobodan-ilic @coveralls @scanny @ernestoarbitrio