search

biocore / American-Gut

American Gut open-access data and IPython notebooks
Other
113 stars 81 forks source link

Module2 modularization #25

Closed jwdebelius closed 11 years ago

ElDeveloper commented 11 years ago

:+1: don't have the setup to test this but the code looks sane to me.

The only minor thing I noticed was that generate_otu_signifigance_tables.py make_phyla_plots.py are both executable files.

jwdebelius commented 11 years ago

Should I make them not executable?

adamrp commented 11 years ago

If they're script files, they should be executable, if they're library code, they should not be.

On Wed, Nov 6, 2013 at 3:49 PM, J W Debelius notifications@github.comwrote:

Should I make them not executable?

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/American-Gut/pull/25#issuecomment-27921996 .

ElDeveloper commented 11 years ago

As I said, it's minor, not something that would be a "blocker".

If @wasade confirms this is ready to go then it should be merged.

On Nov 6, 2013, at 3:51 PM, adamrp notifications@github.com wrote:

If they're script files, they should be executable, if they're library code, they should not be.

On Wed, Nov 6, 2013 at 3:49 PM, J W Debelius notifications@github.comwrote:

Should I make them not executable?

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/American-Gut/pull/25#issuecomment-27921996 .

— Reply to this email directly or view it on GitHub.

wasade commented 11 years ago

does this include the fixes from yesterday?

jwdebelius commented 11 years ago

Yes. The pull request merging was a little off, but it should. It also prints percentage with two decimals.

On Nov 6, 2013, at 8:43 PM, Daniel McDonald notifications@github.com wrote:

does this include the fixes from yesterday?

— Reply to this email directly or view it on GitHub.

ElDeveloper commented 11 years ago

Yes it does, @JWDebelius and I resolved the conflicts and made sure those changes made it in.

Thanks!

Yoshiki Vázquez-Baeza

On Nov 6, 2013, at 8:43 PM, Daniel McDonald notifications@github.com wrote:

does this include the fixes from yesterday?

— Reply to this email directly or view it on GitHub.