broadinstitute / oncotator

Other
67 stars 33 forks source link

Issue 325 First set of OxoG changes (allowing annotation overwriting and intelligent overwriting in TCGA MAF outputs) #327

Closed LeeTL1220 closed 9 years ago

LeeTL1220 commented 9 years ago

This pull request is fairly large.

This implements the easy re-annotation of TCGA MAF files (-i TCGAMAF -o TCGAMAF). Preferably, the datasources should be the same as used for the input TCGA MAF, though this is not required.

Closes #325

More specifics:

LeeTL1220 commented 9 years ago

@dheiman FYI... When this is reviewed and merged, I will cut a release (and update the FH module).

dheiman commented 9 years ago

Awesome, thanks!

On Mon, Jul 13, 2015 at 1:47 PM, Lee Lichtenstein notifications@github.com wrote:

@dheiman https://github.com/dheiman FYI... When this is reviewed and merged, I will cut a release (and update the FH module).

— Reply to this email directly or view it on GitHub https://github.com/broadinstitute/oncotator/pull/327#issuecomment-121003982 .

LeeTL1220 commented 9 years ago

@dheiman I am pretty close to completing the next pull request (which depends on this one as well). I would like to finish that one, get it reviewed, then release. Effectively, this will probably get you your functionality at a cost of little to no time.

lbergelson commented 9 years ago

@LeeTL1220 I didn't get a chance to take a proper look at this yet. I'll do it first thing tomorrow.

LeeTL1220 commented 9 years ago

@lbergelson No prob. I should have the next one almost ready to go. I just need to confirm it still works after this one is merged into it.

lbergelson commented 9 years ago

It looks like MutUtils.createFieldsMapping should be deleted and replaced by FieldMapCreator.create_field_map in all instances unless there is a reason to keep the other around.

lbergelson commented 9 years ago

@LeeTL1220 Review complete. Minor comments, looks good.

LeeTL1220 commented 9 years ago

Removed MutUtils.createFieldMapping and updated other classes appropriately.

LeeTL1220 commented 9 years ago

Running unit tests now.....

LeeTL1220 commented 9 years ago

@lbergelson Changes have been pushed to address everything above, except where noted in conversation above. All unit tests pass on my laptop.