YuLab-SMU / treeio

:seedling: Base Classes and Functions for Phylogenetic Tree Input and Output
https://yulab-smu.top/treedata-book/
94 stars 24 forks source link

Add filter method to get.placement() #85

Open mjchen1996 opened 2 years ago

mjchen1996 commented 2 years ago

Add filter methods for placement filtering: "all","max_lwr","max_pendant", "min_likelihood","lwr","pendant","likelihood"

GuangchuangYu commented 2 years ago

为什么要把summarize_placement()删掉?

mjchen1996 commented 2 years ago

为什么要把summarize_placement()删掉?

老师,我发现summarize_placement()调用了get.placement()是写死的,我想不到要怎么修改比较好,就先删掉了。

GuangchuangYu commented 2 years ago

没想好,那就不要动,这不是显而易见的吗。

写死不是重点,重点是文件解析时,给出了基本的信息(这就可以是写死的一块),然后又有探索其它信息的可能性(这就是活的了)。

mjchen1996 commented 2 years ago

没想好,那就不要动,这不是显而易见的吗。

写死不是重点,重点是文件解析时,给出了基本的信息(这就可以是写死的一块),然后又有探索其它信息的可能性(这就是活的了)。

老师,是这样的,首先我在get.placement()里添加了其他的过滤方法,但我发现在summarize_placement() 调用get.placements()使用了固定的一种方法,by =="best"即为修改后的by == "min_likelihood",这样我添加的其他方法无法应用在summarize_placement() 中,进而无法应用在read.jplace()中,而我不想去改动read.jplace的参数。另外,也考虑到解析jplace文件后,通过get.placements()获取placement的信息,再传入data进去就可以开始下游的可视化了。

GuangchuangYu commented 2 years ago

data哪是可以传来传去的。所谓封装就是让人看不见。

GuangchuangYu commented 2 years ago
  1. 如果有必要的话,搞多个参数,也是可以的。
  2. 如果要像你说的那样,还把数据写回对象里,那应该是写replace method。就是说替换的操作是由一个replace method来完成的,替换了什么slot,对用户是不可见的。不要动不动就:来我告诉你,你用xx@xx然后怎么样怎么样。这是不对的。
  3. 你可以考虑其他的方式。

不管是什么样子的,首先是要想好,想清楚。不能说,我还没想好,所以我把代码注释掉一块。

如何呈现给用户是最主要的,至于怎么实现,都是次要的,你可以propose一些东西,我觉得OK了,你搞不了,让别的学生来搞也没问题。

最重要的一点,想清楚。想清楚。想清楚。

mjchen1996 commented 2 years ago

好的,老师,我再想想。