hammerlab / cohorts

Utilities for analyzing mutations and neoepitopes in patient cohorts
Apache License 2.0
20 stars 4 forks source link

Pretty field labels for functions #119

Open jburos opened 7 years ago

jburos commented 7 years ago

Taking (now closed) PR #116 & turning it into a feature request for discussion.

Goal

Rather than printing missense_snv_col in plots, print Missense SNV Count / MB or Missense SNV Count, depending on the params.

however

(here quoting from the PR)

This got a little complicated for the following reasons:

Once I rename a column, something like cohort.plot_boolean(on=[missense_snv_count, split_3_mos], plot_col="missense_snv_count", boolean_col="split_3_mos") won't work because "split_3_mos" is renamed.

It's non-trivial to figure out whether a function is normalizing per MB. I can hackishly inspect the arguments (fn_argnames = fn.code.co_varnames; if "normalized_per_mb" in fn_argnames), which works for e.g. snv_count, but that doesn't work for expressed_neoantigen_count, for which normalized_per_mb isn't actually an argument. I'll probably need to run the function and set state if we're normalizing.

Going to table this for now.

arahuja commented 7 years ago

I think this resolved (or at least a slightly different discussion) with the new API. Now every plotting function should take a dictionary similar to { 'Nice Name' : compute_func }

jburos commented 7 years ago

Should we allow the "pretty name" versions of fields to be set at the cohort level?

arahuja commented 7 years ago

Should we allow the "pretty name" versions of fields to be set at the cohort level?

I think for functions that we provide that could make sense, since we know all of those functions, i.e. snv_count, missense_snv_coun, but would hard in general, since there is not a complete collection of possible functions.

jburos commented 7 years ago

Yeah that makes sense. I was thinking of allowing a user to pass in a dictionary of pretty names on cohort creation which, if defined for a function, could be the default display name.

tavinathanson commented 7 years ago

At first glance I like that idea @jburos 👍