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

should --i-feature-table be optional? #175

Closed
antgonza opened this issue Jun 16, 2020 · 6 comments · Fixed by #375
Closed

should --i-feature-table be optional? #175

antgonza opened this issue Jun 16, 2020 · 6 comments · Fixed by #375
Assignees
Milestone

Comments

@antgonza
Copy link
Collaborator

In a current use case, I do not care about the feature table and only about the rep-set and the rep-set metadata; thus, wondering if --i-feature-table should be optional. Thoughts?

@fedarko
Copy link
Collaborator

fedarko commented Jun 16, 2020

We just discussed this this morning. With the work @ElDeveloper has done in #174 on defining an Empress "object" (#140), it will be a lot easier to make new visualizer commands. IIRC we settled on having at minimum two options, something like

  1. qiime empress community-tree-plot, which requires at minimum a tree, table, and sample metadata (analogous to how Empress currently works, where the focus is on showing how a given dataset looks overlaid on a tree)

  2. qiime empress tree-plot, which would require only a tree

In both of the above cases, feature metadata would be optional. (Visualizer names are obvs tentative.)

The motivation for splitting this into multiple visualizers, rather than just one visualizer with an optional table input, is that having the table would also require having sample metadata (to make the table useful).

@ElDeveloper
Copy link
Member

This was brought up again as potentially being useful for visualization COVID-19 related workflows in other labs.

cc @ebolyen

@fedarko fedarko self-assigned this Sep 1, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 1, 2020
-TODO polish interface more, hide more stuff (barplots / sm menu)
-TODO clean up code
-TODO test
fedarko added a commit to fedarko/empress that referenced this issue Sep 1, 2020
Still need to add tons of tests and iron out bugs but this should
close biocore#175
@fedarko
Copy link
Collaborator

fedarko commented Sep 1, 2020

There's a prototype version of this available in this branch. Current interface (subject to change) has two visualizers, as described above:


$ qiime empress
Usage: qiime empress [OPTIONS] COMMAND [ARGS]...

  Description: This QIIME 2 plugin wraps Empress and supports interactive
  visualization of phylogenetic trees.

  Plugin website: http://github.com/biocore/empress

  Getting user support: Please post to the QIIME 2 forum for help with this
  plugin: https://forum.qiime2.org

Options:
  --version    Show the version and exit.
  --citations  Show citations and exit.
  --help       Show this message and exit.

Commands:
  community-plot  Visualize phylogenies and community data with Empress (and,
                  optionally, Emperor)
  tree-plot       Visualize phylogenies with Empress

$ qiime empress tree-plot --help
Usage: qiime empress tree-plot [OPTIONS]

  Generates an interactive phylogenetic tree visualization supporting
  interaction with feature metadata.

Inputs:
  --i-tree ARTIFACT    The phylogenetic tree to visualize.
    Phylogeny[Rooted]                                               [required]
Parameters:
  --m-feature-metadata-file METADATA...
    (multiple          Feature metadata. Can be used to color nodes (tips
     arguments will    and/or internal nodes) in the tree. Features described
     be merged)        in the metadata that are not present in the tree will
                       be automatically filtered out of the visualization.
                                                                    [optional]
Outputs:
  --o-visualization VISUALIZATION
                                                                    [required]
Miscellaneous:
  --output-dir PATH    Output unspecified results to a directory
  --verbose / --quiet  Display verbose output to stdout and/or stderr during
                       execution of this action. Or silence output if
                       execution is successful (silence is golden).
  --citations          Show citations and exit.
  --help               Show this message and exit.

$ qiime empress community-plot --help
Usage: qiime empress community-plot [OPTIONS]

  Generates an interactive phylogenetic tree visualization supporting
  interaction with sample and feature metadata and, optionally, Emperor
  integration.

Inputs:
  --i-tree ARTIFACT    The phylogenetic tree to visualize.
    Phylogeny[Rooted]                                               [required]
  --i-feature-table ARTIFACT FeatureTable[Frequency]
                       A table containing the abundances of features within
                       samples. This information allows us to decorate the
                       phylogeny by sample metadata. It's expected that all
                       features in the table are also present as tips in the
                       tree, and that all samples in the table are also
                       present in the sample metadata file.         [required]
  --i-pcoa ARTIFACT    Principal coordinates matrix to display simultaneously
    PCoAResults        with the phylogenetic tree using Emperor.    [optional]
Parameters:
  --m-sample-metadata-file METADATA...
    (multiple          Sample metadata. Can be used to color tips in the tree
     arguments will    by the samples they are unique to. Samples described in
     be merged)        the metadata that are not present in the feature table
                       will be automatically filtered out of the
                       visualization.                               [required]
  --m-feature-metadata-file METADATA...
    (multiple          Feature metadata. Can be used to color nodes (tips
     arguments will    and/or internal nodes) in the tree. Features described
     be merged)        in the metadata that are not present in the tree will
                       be automatically filtered out of the visualization.
                                                                    [optional]
  --p-ignore-missing-samples / --p-no-ignore-missing-samples
                       This will suppress the error raised when the feature
                       table contains samples that are not present in the
                       sample metadata. Samples without metadata are included
                       in the visualization by setting all of their metadata
                       values to "This sample has no metadata". Note that this
                       flag will only be applied if at least one sample is
                       present in both the feature table and the metadata.
                                                              [default: False]
  --p-filter-extra-samples / --p-no-filter-extra-samples
                       This will suppress the error raised when samples in
                       the feature table are not included in the ordination.
                       These samples will be will be removed from the
                       visualization if this flag is passed. Note that this
                       flag will only be applied if at least one sample in the
                       table is also present in the ordination.
                                                              [default: False]
  --p-filter-missing-features / --p-no-filter-missing-features
                       This will suppress the error raised when the feature
                       table contains features that are not present as tips in
                       the tree. These features will be removed from the
                       visualization if this flag is passed. Note that this
                       flag will only be applied if at least one feature in
                       the table is also present as a tip in the tree.
                                                              [default: False]
  --p-number-of-features INTEGER
    Range(1, None)     The number of most important features (arrows) to
                       display in the ordination. "Importance" is calculated
                       for each feature based on the vector’s magnitude
                       (euclidean distance from origin). Note, this parameter
                       is only honored when a biplot is inputed.  [default: 5]
  --p-filter-unobserved-features-from-phylogeny /
  --p-no-filter-unobserved-features-from-phylogeny
                       If this flag is passed, filters features from the
                       phylogeny that are not present as features in feature
                       table. Default is True.                 [default: True]
Outputs:
  --o-visualization VISUALIZATION
                                                                    [required]
Miscellaneous:
  --output-dir PATH    Output unspecified results to a directory
  --verbose / --quiet  Display verbose output to stdout and/or stderr during
                       execution of this action. Or silence output if
                       execution is successful (silence is golden).
  --citations          Show citations and exit.
  --help               Show this message and exit.

@ElDeveloper
Copy link
Member

Fairly personal opinion: after seeing this in a more tangible form I think keeping just qiime empress plot with the feature table as an optional argument makes more sense. Other thoughts?


Thanks for putting this together!

@fedarko
Copy link
Collaborator

fedarko commented Sep 1, 2020

Thanks!

I would really prefer to keep this split up into two separate commands, because making the feature table / sample metadata optional would mean that it'd be possible for the user to pass one of those but not both of them. This seems to me like it would confuse users.

Without the table the sample metadata isn't useful (no way to map the sample metadata groups to the tree tips), and without the sample metadata the table isn't useful (no way to interpret the meanings of samples, unless I guess we're showing an Emperor plot with sample metadata -- in which case the sample metadata should have been passed to Empress in the first place).

We could keep everything as one command and then add some code to raise an error if the user provides only one of the table and sample metadata, but honestly that seems like it would complicate the interface too much.

@ElDeveloper
Copy link
Member

ElDeveloper commented Sep 1, 2020 via email

fedarko added a commit to fedarko/empress that referenced this issue Sep 3, 2020
this was already annoying with the feature metadata controls;
now it's nicer
fedarko added a commit to fedarko/empress that referenced this issue Sep 9, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 10, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 10, 2020
fedarko added a commit to fedarko/empress that referenced this issue Sep 10, 2020
also refactored the legend clear method to use jquery bc why not
fedarko added a commit to fedarko/empress that referenced this issue Sep 10, 2020
For biocore#175 -- avoids confusion in just-sm / just-fm plots.

We could also dynamically change this text based on how much data
was passed to Empress but ... that's a lotta work for marginal benefit
ElDeveloper added a commit that referenced this issue Sep 25, 2020
)

* ENH: Initial stab at "just tree/fm" plots #175

-TODO polish interface more, hide more stuff (barplots / sm menu)
-TODO clean up code
-TODO test

* ENH: improve UI hiding re: level of data passed

* ENH/BUG: make just-fm tree plots work ok

Still need to add tons of tests and iron out bugs but this should
close #175

* STY: linty mclintface

* DOC: add example usages to makefile

todo should update readme

* DOC: update readme re community plot stuff

* DOC: reword s.m. desc in community-plot

since "tip-level animations" not what i was going for

* ENH: accept different featuretable types+upd8 docs

* TST: unbreak core tests

* TST: unbreak + add basic integration tests

* DOC/TST: mention limitations of curr integ tests

* TST: test that comm plot reqs both tbl and sm

* MNT: update Makefile and Travis stuff

* MNT: abstract reused code btwn comm & tree plot

* MNT: rename newickpath -> newickfmt to be clearer

* DOC: add note re: 2 fm matching funcs

(pls don't make me combine them ._____________________________.)

* MNT: remove redundant var in empress-template html

* MNT: simplify small qzv names

* ENH: fix flash of unstyled content pbm for #175 UI

this was already annoying with the feature metadata controls;
now it's nicer

* DOC: document _plot() funcs

* ENH: more sophisticated ctrl hiding/showing stuff

... based on how much data is available. Now, there are these
easy-to-add "needs-community-data", "needs-feature-metadata", and
"needs-metadata" classes that we can use to keep track of stuff.

also now rather than hiding stuff by default then unhiding it we
go with showing by default and then hiding; since we now delay showing
the ctrl panel at all until after loading p much done, this is ok

* DOC: lets you -> helps

* TST: test that isCommunityPlot/fmCols set ok #175

* STY: put big url on own line

* TST: add tbl/sm none-chking to core init&test #175

* TST: test tree-plot fm stuff in more detail

* TST: check fm filtering for tree-plot

* TST test to_dict for tree plot #175

* TST: expand to_dict tests for tree plot

* DOC: add note to core init abt tree plot junk

* TST/DOC: rm note about TODO which I todid

* TST: test some barplot panel JS #175 stuff

also refactored the legend clear method to use jquery bc why not

* TST: test getSampleCategories() if not comm plot

* DOC: clarify split commands in README

also added some extra context to first paragraph

* DOC: "feature and/or sample metadata"->"metadata"

For #175 -- avoids confusion in just-sm / just-fm plots.

We could also dynamically change this text based on how much data
was passed to Empress but ... that's a lotta work for marginal benefit

* BUG: Actually, don't accept FT[Composition] inputs

Since we convert tables to presence/absence anyway, pseudocounts
will explode that.

* STY: rm now-unused import of Composition

* DOC: Rename / polish docs for shearing option

Closes #312.

Old: --p-filter-unobserved-features-from-phylogeny (45 chars)
New: --p-shear-to-table                            (18 chars)

* TST: update make-dev-page re: new flag name #312

* STY: flake8

* TST: unbreak tests due to #312 change

* DOC: update readme re new shearing param name

also tidy up subset table section

* DOC: clarify filtered tbl discussion more

* ENH: Make legend export UI require metadata

Addresses @ElDeveloper comment.

* Update .travis.yml

Co-authored-by: Yoshiki Vázquez Baeza <[email protected]>

* Update empress/core.py

Co-authored-by: Yoshiki Vázquez Baeza <[email protected]>

* DOC: remove old HTML todo

thanks @ElDeveloper for catching!

* ENH: Rename --p-shear-to-table to --p-shear-tree

Based on discussion with @ElDeveloper

Co-authored-by: Yoshiki Vázquez Baeza <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants