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.

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

Statistical power #127

Merged
merged 67 commits into from
Mar 8, 2015
Merged

Statistical power #127

merged 67 commits into from
Mar 8, 2015

Conversation

jwdebelius
Copy link
Contributor

This is a cleaned up power notebook, loosely related to what was presented in #121

The rendered notebook can be found here.

The text is in Google docs, which may be a better place for text editing.

This notebook relies on code generated in #125 and on the data files generated when #126 is run.

@jwdebelius
Copy link
Contributor Author

@wasade, I've updated the code as per your suggestions and added the new text to the notebook.


# Prevents the confidence interval of being less than 0 or more than 1
pwr_lower = pwr_mean - pwr_bound
np.where(pwr_lower < 0, 0, pwr_lower)
Copy link
Member

Choose a reason for hiding this comment

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

This is not an inplace operation, so this doesn't have an effect. What you want here is pwr_lower = np.where(pwr_lower < 0, 0, pwr_lower).

Have the changes made on the last round of commits been verified they work as expected?

@wasade
Copy link
Member

wasade commented Mar 5, 2015

Still a few typos I saw with 'significance', and the trace_bounds method has some method calls that are currently not doing anything. Going over the rendered notebook now

@wasade
Copy link
Member

wasade commented Mar 5, 2015

@ElDeveloper, were you able to go over the other notebook by chance?

@ElDeveloper
Copy link
Member

No, I wasn't able to go over the other notebook.

On (Mar-05-15|15:57), Daniel McDonald wrote:

@ElDeveloper, were you able to go over the other notebook by chance?


Reply to this email directly or view it on GitHub:
#127 (comment)

@wasade
Copy link
Member

wasade commented Mar 6, 2015

Just on a quick pass through, the rendered notebook still has grammar and spelling issues to be resolved. Where the suggestions that @cuttlefishh and @EmbrietteH made to the google doc merged in?

@wasade
Copy link
Member

wasade commented Mar 6, 2015

Possible to do so?

On Thu, Mar 5, 2015 at 5:00 PM, Yoshiki Vázquez Baeza <
notifications@github.com> wrote:

No, I wasn't able to go over the other notebook.

On (Mar-05-15|15:57), Daniel McDonald wrote:

@ElDeveloper, were you able to go over the other notebook by chance?


Reply to this email directly or view it on GitHub:
#127 (comment)


Reply to this email directly or view it on GitHub
#127 (comment).

@jwdebelius
Copy link
Contributor Author

I tested the code by running it through both notebooks, and the test suite.
The suggestions were merged in last night. Will run through spell check again.

@wasade
Copy link
Member

wasade commented Mar 6, 2015

From what I'm seeing, the notebook text does not appear to be in sync with
the google doc. I mashed refresh a few times, so it shouldn't be a cached
copy

On Thu, Mar 5, 2015 at 5:12 PM, J W Debelius notifications@github.com
wrote:

I tested the code by running it through both notebooks, and the test suite.
The suggestions were merged in last night. Will run through spell check
again.


Reply to this email directly or view it on GitHub
#127 (comment).

@jwdebelius
Copy link
Contributor Author

That is the updated notebook and text. What I'm seeing lines up with the
google doc, when I click both links.

On Thu, Mar 5, 2015 at 4:17 PM, Daniel McDonald notifications@github.com
wrote:

From what I'm seeing, the notebook text does not appear to be in sync with
the google doc. I mashed refresh a few times, so it shouldn't be a cached
copy

On Thu, Mar 5, 2015 at 5:12 PM, J W Debelius notifications@github.com
wrote:

I tested the code by running it through both notebooks, and the test
suite.
The suggestions were merged in last night. Will run through spell check
again.


Reply to this email directly or view it on GitHub
<#127 (comment)
.


Reply to this email directly or view it on GitHub
#127 (comment).

@ElDeveloper
Copy link
Member

This is the reason why I can't comment on the notebook:
https://github.com/biocore/American-Gut/pull/127/files#diff-c6b18319a5fc61e394924c6a0a3ff2d3

GitHub will not show the interface for me to make comments on it.

On (Mar-05-15|16:10), Daniel McDonald wrote:

Possible to do so?

On Thu, Mar 5, 2015 at 5:00 PM, Yoshiki Vázquez Baeza <
notifications@github.com> wrote:

No, I wasn't able to go over the other notebook.

On (Mar-05-15|15:57), Daniel McDonald wrote:

@ElDeveloper, were you able to go over the other notebook by chance?


Reply to this email directly or view it on GitHub:
#127 (comment)


Reply to this email directly or view it on GitHub
#127 (comment).


Reply to this email directly or view it on GitHub:
#127 (comment)

@wasade
Copy link
Member

wasade commented Mar 6, 2015

@jwdebelius, please see the screenshots below. There are others as well

@ElDeveloper, able to comment separate from inline on github? It's not ideal, but we need to review the material somehow

screen shot 2015-03-05 at 5 26 48 pm
screen shot 2015-03-05 at 5 27 00 pm

*Redirect works, but the analysis was run with a table already
processed for rounds 1-14
@jwdebelius
Copy link
Contributor Author

I've updated. I can ask someone else to check through spelling again before you give another pass.

Also, I processed the data for the alpha diversity on a pre-computed tables, since the current maps are missing preceding zeros. This skips the download step, since the notebook requires the github directory, but assumes the kernel is running in the ipnyb directory.

@wasade
Copy link
Member

wasade commented Mar 6, 2015

Thanks. I'm going to do one more pass through today. I think we're nearly or already there, so it should go quick. And its not like we can't issue future PRs to update if necessary anyway. Thank you for all the hard work on this!!

@jwdebelius
Copy link
Contributor Author

I asked an extra person to look through it. I thought they'd be done tonight, but they sent me a couple more edits that I'm working on now. Sorry for the lengthy process.

Hopefully this is the last round
@jwdebelius
Copy link
Contributor Author

Okay, I got the new stuff in, too. Should be good.

@wasade
Copy link
Member

wasade commented Mar 7, 2015

Sorry, didn't see the comment until now. Reviewing

@wasade
Copy link
Member

wasade commented Mar 8, 2015

I think this is good, thanks @jwdebelius!!!!

wasade added a commit that referenced this pull request Mar 8, 2015
@wasade wasade merged commit 93ff186 into biocore:master Mar 8, 2015
@jwdebelius jwdebelius deleted the statistical_power branch March 10, 2015 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants