[Phylobase-commits] r584 - pkg/R

[Jim Regetz]

Hmm, I don't like that internal coercion now causes the warning to pop 
up when using subset on "unknown"-ordered phylo, phylo4, or phylo4d 
objects (twice), and when using prune on either phylo4 or phylo4d.

Internally, ape::drop.tip (which ultimately gets called by both subset 
and prune) currently reorders trees as cladewise, so AFAIK no matter 
what you start with, you're going to get a "cladewise" tree back in the 
case of subset("phylo") or prune("phylo"), and a "preorder" tree back in 
the case of subset("phylo4[d]") or prune("phylo4[d]"). Maybe the subset 
and prune methods should just do an explicit reorder before getting on 
with the rest of it? Kind of wasteful insofar as this reorder would 
serve no purpose other than to avoid a warning, but it should work 
(untested).

I think I'm okay with a warning if the issue above gets fixed, and if 
warnings only surface when someone deliberately chooses to export phylo4 
-> phylo. That said, if we choose to be more aggressive about reordering 
by default, that's fine too (unless there are performance concerns). 
Indeed, we could still have round-trip contract stating that 
all.equal.phylo(tr.in, tr.out) will return TRUE, but not necessarily 
identical(tr.in, tr.out) nor all.equal.default(tr.in, tr.out):

Jim

Peter D. Cowan wrote:
> I think I replied only to Ben previously, perhaps reorder at default, and an option to maintain order?
> 
> Peter
> 
> On Mon, Aug 24, 2009 at 08:01:40PM -0700, Steve Kembel wrote:
>> I'm also in favour of not automatically reordering trees on phylo <>
>> phylo4 conversion. There are cases where I would really like to
>> maintain tree order (because I have some information about nodes that
>> is in Newick string order).
>>
>> I am not sure of a better solution. Am I correct that unknown ordering
>> is only really a potential problem on export *to* ape where it could
>> break some functions (i.e. plotting)? Could we warn in this case but
>> not on phylo -> phylo4 conversion?
>>
>> -Steve
>> On Mon, Aug 24, 2009 at 7:23 PM, Ben Bolker<bolker at ufl.edu> wrote:
>>>  hmmm.
>>>  you may be right, but this will presumably break round-trip capability
>>> -- i.e. if we import a tree with unknown ordering and automatically
>>> re-order it when we export, the round trip will have changed it.
>>> Am I being too obsessive about this?  Is there a better solution?
>>>
>>>  Ben
>>>
>>> Peter D. Cowan wrote:
>>>> On Tue, Aug 25, 2009 at 02:58:59AM +0200, noreply at r-forge.r-project.org wrote:
>>>>> Author: bbolker
>>>>> Date: 2009-08-25 02:58:59 +0200 (Tue, 25 Aug 2009)
>>>>> New Revision: 584
>>>>>
>>>>> Modified:
>>>>>    pkg/R/setAs-Methods.R
>>>>> Log:
>>>>>   added error if unknown ordering imported from ape
>>>>>   added warning if unknown ordering exported to ape
>>>>>
>>>> How about reordering unknown trees before exporting them?  This seems likely to flood users with warnings.
>>>>
>>>> peter
>>>>
>>>>> Modified: pkg/R/setAs-Methods.R
>>>>> ===================================================================
>>>>> --- pkg/R/setAs-Methods.R    2009-08-25 00:51:06 UTC (rev 583)
>>>>> +++ pkg/R/setAs-Methods.R    2009-08-25 00:58:59 UTC (rev 584)
>>>>> @@ -26,11 +26,11 @@
>>>>>          }
>>>>>      }
>>>>>      oldorder <- attr(from,"order")
>>>>> -    neworder <- if (is.null(oldorder)) { "unknown" } else {
>>>>> -      switch(oldorder,
>>>>> -             pruningwise="pruningwise",
>>>>> -             cladewise="preorder")
>>>>> -    }
>>>>> +    neworder <- if (is.null(oldorder)) { "unknown" } else
>>>>> +    if (!oldorder %in% phylo4_orderings) {
>>>>> +      stop("unknown ordering '",oldorder,"' in ape object")
>>>>> +    } else if (oldorder=="cladewise") "preorder"
>>>>> +    else oldorder
>>>>>      attr(from,"order") <- NULL
>>>>>      newobj <- phylo4(from$edge, from$edge.length, from$tip.label,
>>>>>                       node.label = from$node.label,
>>>>> @@ -113,9 +113,10 @@
>>>>>                                     preorder  = 'cladewise',
>>>>>                                     unknown = 'unknown',
>>>>>                                     pruningwise = 'pruningwise')
>>>>> -    } else {
>>>>> -      ## warning ??
>>>>> -    }
>>>>> +      } else {
>>>>> +        ## warning ??
>>>>> +        warning("trees with unknown order may be unsafe in ape")
>>>>> +      }
>>>>>      if (length(y$edge.length) == 0)
>>>>>          y$edge.length <- NULL
>>>>>      if (length(y$node.label) == 0)
>>>>>
>>>>> _______________________________________________
>>>>> Phylobase-commits mailing list
>>>>> Phylobase-commits at lists.r-forge.r-project.org
>>>>> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/phylobase-commits
>>>
>>> --
>>> Ben Bolker
>>> Associate professor, Biology Dep't, Univ. of Florida
>>> bolker at ufl.edu / www.zoology.ufl.edu/bolker
>>> GPG key: www.zoology.ufl.edu/bolker/benbolker-publickey.asc
>>>
>>>
>>> _______________________________________________
>>> Phylobase-commits mailing list
>>> Phylobase-commits at lists.r-forge.r-project.org
>>> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/phylobase-commits
>>>
>>>
>>
>>
>> -- 
>> ______________________________________________
>> Dr. Steven Kembel - steve.kembel at gmail.com
>> http://www.phylodiversity.net/skembel/
> _______________________________________________
> Phylobase-commits mailing list
> Phylobase-commits at lists.r-forge.r-project.org
> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/phylobase-commits
>