search

biocore / my-microbes

A set of tools for delivering personal microbiome results to individuals participating in microbiome sequencing studies.
7 stars 5 forks source link

Add explicit time axis to beta diversity plots #34

Closed ElDeveloper closed 11 years ago

ElDeveloper commented 11 years ago

Should resolve the second task of issue #22.

gregcaporaso commented 11 years ago

@jrrideout, this looks OK to me (pending responses to my two comments) but do you want to review this one as you've been working on the code more than I have over the last few weeks?

gregcaporaso commented 11 years ago

@ElDeveloper, what's your ETA for these changes? @jrrideout and I are working on some other changes now and we'd like to wrap up the first round of development today.

ElDeveloper commented 11 years ago

I have a working version of this minus the updates to the test module, regarding this, what would be the way to test create_personal_mapping_file, I see some comments questioning this as well. I was thinking:

jairideout commented 11 years ago

I'm currently working on adding unit tests for util.py and have added tests for create_personal_mapping_file(). Don't worry too much about adding unit tests for now, since I'm going to be finishing them up fairly soon (unless there's something relatively isolated in your changes that you want to specifically test).

ElDeveloper commented 11 years ago

Ok that's cool. The only thing is that the tests will fail with my changes in place, so I would have to modify the tests you are adding.

I'm currently on my phone, will be back in my computer in 30-40 mins.

Thanks!

El martes, 22 de enero de 2013, Jai Ram Rideout escribió:

I'm currently working on adding unit tests for util.py and have added tests for create_personal_mapping_file(). Don't worry too much about adding unit tests for now, since I'm going to be finishing them up fairly soon (unless there's something relatively isolated in your changes that you want to specifically test).

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/my-microbes/pull/34#issuecomment-12568839.

jairideout commented 11 years ago

That's fine, go ahead and push your latest changes when they're ready and I'll take a look.

ElDeveloper commented 11 years ago

Ok, thanks for that, I've just pushed the latest updates accordingly to what was mentioned before.

jairideout commented 11 years ago

Looks like I can't merge automatically- can you please fix and push again? Thanks!

ElDeveloper commented 11 years ago

Sure.

ElDeveloper commented 11 years ago

Wow, there are quite a few conflicts, in favor of me not screwing anything, would you be ok if I issued a new pull request with only the latest changes added?

jairideout commented 11 years ago

Sure

On Tue, Jan 22, 2013 at 5:23 PM, Yoshiki notifications@github.com wrote:

Wow, there are quite a few conflicts, in favor of me not screwing anything, would you be ok if I issued a new pull request with only the latest changes added?

— Reply to this email directly or view it on GitHubhttps://github.com/qiime/my-microbes/pull/34#issuecomment-12575216.