Transformer from Newick to dendropy

[chrysn]

Moving code from Makefile driven exports and usage of the files from Python to directly accessing, I found that no conversion from a Phylogeny[Rooted] artifact to a dendropy tree is provided.

I'd like to bridge this gap, but before I shape this into a full PR I'd like to obtain clarity on two points:

• Is this the right module to add this in?

  I think so, because the skbio.TreeNode transformation sits here too -- but then again, skbio appears to be an unconditional dependency of q2-types already. Could dendropy be a weak dependency in that the conversion is just not registered if dendropy fails to import? (After all, a user can not call Artifact.load(...).view(dendropy.Tree) if they can't import dendropy, it wouldn't get noticed).

• Dendropy differentiates between rooted and unrooted trees, but in the conversion all I see is NewickFormat, and not the Phylogeny[Rooted] artifact type that would be the base for that decision. Is there an established way to obtain the type in a transformer? Or is there a different mechanism than running a transformer that is more suitable for preserving this metadatum?

Current stand-alone code for refernce

(Most of this is boilerplate code that wouldn't be needed when placed at, say tree/_transformer.py in here).

import dendropy
import qiime2
from qiime2.sdk import PluginManager
from q2_types.tree import NewickFormat

plugin = qiime2.plugin.Plugin('newick_to_dendropy', version="0", website="na")

@plugin.register_transformer
def _load(ff: NewickFormat) -> dendropy.Tree:
    # FIXME assert ff is RootedTree, or better introspect and decide the rooting
    with ff.open() as fh:
        return dendropy.Tree.get(file=fh, schema='newick', rooting='default-rooted')

PluginManager().add_plugin(plugin, package='manual', project_name='manual')

[ebolyen]

Hi @chrysn!

Is this the right module to add this in?

Yep!

Could dendropy be a weak dependency in that the conversion is just not registered if dendropy fails to import?

That's a pretty interesting idea. I suppose it's absolutely possible to wrap the transformer registrations behind a test for the dendropy import.

It's also totally fine to just make it a dependency. We would start by adding it to the recipe in an initial PR, once that is merged and flushes through our CI system (takes about half a day), we can create the second PR with your changes which include the transformers.

Is there an established way to obtain the type in a transformer? Or is there a different mechanism than running a transformer that is more suitable for preserving this metadatum?

I believe we use the convention of 3 children on the root node to mean unrooted, and bifurcating to mean rooted. I realize it isn't the strictest thing, but that might be useful.

Otherwise, there's not anything available in the transformer for this (although I think there used to be an unconnected parameter that imagined it could be used for that sort of thing). It sounds like this might be a compelling reason to add something like that. I imagine it would be something like:

@plugin.register_transformer(context=Phylogeny[Rooted])
def _whatever(ff: ...) -> Tree:
   # now we know it's rooted, and would make another transformer for unrooted

Our transformation system would need quite the overhaul (which is frankly overdue) to pull that off though.