Hi Pjotr,
I'm reading the code (commit c2de9dd3ad055bab4bfb1d3e8da840493b110b0e).
It is generally good. Below are my comments and suggested changes.
> # == Examples
> #
> # Read the codeml M0-M3 data file into a buffer
> #
> # >> require 'bio/test/biotestfile'
> # >> buf = BioTestFile.read('paml/codeml/models/results0-3.txt')
It is not suitable to use such nonstandard class in the example.
Users want to know the example usage and do not intend to test.
Note that I still disagree with the BioTestFile class.
> class Report < Bio::PAML::Common::Report
>
> attr_reader :models, :header, :footer
RDoc documentation is also needed for attributes. To write RDoc,
the three attribute definitions are needed to be separated.
For example,
# Models in the result
# (Array containing Bio::PAML::Codeml::Model objects)
attr_reader :models
# ...(should be written)
attr_reader :header
# ...(should be written)
attr_reader :footer
> # Parse codeml output file passed with +buf+
> def initialize buf
Details of +buf+ (class, contents, etc) should also be written in RDoc.
It is recommended to use the style written in the README_DEV.rdoc, or
the style used in the Ruby source code.
Please do not omit parentheses in the method definition lines.
> # Model class
> class Model
Too few documentation. At least please write a message that it is
created by Bio::PAML::Codeml::Report.
> def initialize buf
Please write RDoc that normal users do not use the method directly,
and internally called inside the Bio::PAML::Codeml::Report objects.
Please do not omit parentheses in the method definition lines.
> def lnL
Writing RDoc document is needed. In addition, for omega, kappa, alpha,
tree_length, tree, and to_s methods.
> class PositiveSite
Almost all methods have no RDoc documantation.
> def to_a
> [ @position, @aaref, @probability, @omega ]
> end
What is the purpose of the method?
> class PositiveSites < Array
To inherit Array and to create original container class is discouraged.
In BioRuby, we have deprecated Bio::Features and Bio::References in
version 1.3.0, although they do not inherit Array but have an array
in the object. (The classes still exist only for backward compatibility,
in lib/bio/compat/features.rb and references.rb).
In this case, except initialize, only a method named "graph" is added.
I think it is good to add the graph method in the Report class and
using an Array for storing PositiveSite objects.
Naohisa Goto
ngoto at gen-info.osaka-u.ac.jp / ng at bioruby.org