[BioRuby] restriction enzyme module

[Trevor Wennblom]

Hi Toshiaki,

Thanks for looking over my code.  It's very helpful to have another  
pair of eyes.


== 1 ==

 > Firstly, your code is well documented but you have a lot of
 > sub classes under the Bio::RestrictionEnzyme name space.
 > It looks very complicated and I still don't understand which
 > class to use depending on the situations.
 > Could you describe the typical use cases?
 > I hope to include your description in the tutorial, if possible.

The only classes I'm primarily concerned about exposing and having  
the API well documented at the moment are Bio::RestrictionEnzyme and  
Bio::RestrictionEnzyme::Analysis which are both documented in the  
Bio::RestrictionEnzyme RDoc.  This could use some more examples and  
I'm putting together improved documentation as Pjotr and I are using  
this in some real-world tools.

Everything else is supporting classes, not terribly useful in any  
other context outside of Bio::RestrictionEnzyme, and the underlying  
API and usage might change as I get feedback from users as to what's  
useful and what's not.  The underlying structure itself is very  
detailed to account for an amazing amount of flexibility which I'll  
be happy to flesh out if there are individuals who would benefit from  
having the nitty-gritty internals exposed as a more user-friendly API.

This is the kind of thing that I'd shoot for in a BioRuby 1.1 minor  
point release in the future.


== 2 ==

 > Secondly, I don't like to have code to modify the RUBYLIB in the  
library.

done - it was incredibly convenient when designing but now that it's  
incorporated into BioRuby it's not needed.


== 3 ==


 > lib/bio/util/restriction_enzyme/integer.rb
 >
 > is enough to be
 >
 >   if (a != nil and a < 0) or (b != nil and b < 0)
 >
 > How do you think?

done - it was easier on the eyes, but perhaps at a slight speed  
disadvantage.


== 4 ==

 > Why you need 'pp' library here? (It seems that the module is not  
used.)

done - left over from development.


== 5 ==

 > Why this empty class need to be present?

Abstract base class - while not technically necessary I'm strongly in  
favor of keeping it for clarity and further development.


== 6 ==

 > Could you fold your RDoc documents less than 80 columns as long as  
possible?
 > Or should I use larger terminal width...?

Yes to both!  :D

Sometimes I miss that since anytime I modify my comments I have to  
reformat them to be 80 characters wide.  While I don't think it  
should be a requirement, it is something that we should strive for -  
perhaps a "strong suggestion."

 > Or should I use larger terminal width...?

I use a mixture of TextMate and the terminal on OS X.  Just a quick  
glance shows that I currently have terminals open with 147, 80, 175,  
230, and 122 columns wide.  I resize those little guys all the time.


== 7 ==

 > And, you looks to have different strategies for RDoc documentation  
on each file.
 > Which is the best practice?

Thanks for catching a few mistakes of mine.  I think the README.DEV  
is unclear or incorrect in how RDoc works in some circumstances.   
This is something that I wanted to address in a larger document.   
Since it seems like you're interested could you give me a few days to  
put together a well thought-out guide for that, would that be okay?


== 8 ==

 > def cut_with_enzyme(*args)
 >    Bio::RestrictionEnzyme::Analysis.cut(self, *args)
 > end
 > alias cut_with_enzymes cut_with_enzyme
 >
 > When do you plan to move this into lib/bio/sequence.rb?

I wanted to get you're approval first.  Do you think that's the right  
thing to do?  It would be nice since people wouldn't have to include  
the analysis file directly.  Do you forsee any problems with me just  
moving the four lines over to Bio::Sequence::NA?


== 9 ==

Regarding to_re

Did you ever get to_re to work correctly?  I seem to remember you had  
a pretty good patch a while back.  I'm still really itching for an  
official fix since there are many cases where Bio::RestrictionEnzyme  
returns the wrong cut fragments because of incorrect matching.

Presently

   puts Bio::Sequence::NA.new("tan").to_re  # => (?-mix:ta[atgc])

Should be

   puts Bio::Sequence::NA.new("tan").to_re  # => (?-mix:ta[atgcn])


Thanks!
Trevor