Fix: minor bug with input verification in write.phylip; clean up formatting

[ahl27]

Changes the following:

• Minor bug with input verification in write.phylip is now fixed. Previously, calling write.phylip(DNAStringSet("ATGC")) would simply do nothing and return normally. Previous logic would only perform an action if the object inherits from MultipleAlignment, and do nothing otherwise. New logic will throw an error if the object does not inherit from MultipleAlignment, and write the correct data otherwise.
• Numerous formatting fixes throughout MultipleAlignment.R. Cleaned up a lot of trailing space, standardized indents (was previously a mix of 2-space and 4-space indents), cleaned up various other things (e.g. }else{ -> } else {).
• Documentation previously did not mention readXMultipleAlignment(filepath, format='phylip') was supported; this has been fixed.
• Small new unit tests to double check this functionality.

We could also attempt to coerce the input to XMultipleAlignment if it's of type XString or XStringSet, but I'm not sure if we need to do that.

[hpages]

Thanks for catching this bug and for cleaning the messy formatting!

We could also attempt to coerce the input to XMultipleAlignment if it's of type XString or XStringSet, but I'm not sure if we need to do that.

Maybe not needed, I agree. This coercion doesn't seem to be supported anyways:

> as(DNAStringSet("ATGC"), "DNAMultipleAlignment")
Error in as(DNAStringSet("ATGC"), "DNAMultipleAlignment") : 
  no method or default for coercing “DNAStringSet” to “DNAMultipleAlignment”

This kind of reinforces the idea that one should not try to use a DNAStringSet object where a DNAMultipleAlignment object is expected.

Now for the usual nitpicking, sorry 😉:

    if(!inherits(x, "MultipleAlignment")){
        stop("'x' must be an object of class 'MultipleAlignment'")
    }

3 things:

• Even though in my experience inherits() seems to work fine to test inheritance of S4 objects, we're supposed to use is() in this case. I don't know if there are situations where this would make a difference though, I've never seen one. But let's just not take any chance on this.
• x is expected to be a MultipleAlignment object, that is, to be of class 'MultipleAlignment' but not necessarily. In other words, it's expected to be a MultipleAlignment object (from an is() point-of-view) but not necessarily a MultipleAlignment instance (i.e. an object of class 'MultipleAlignment'). All this to say that the error message should simply be "'x' must be a MultipleAlignment object" or "'x' must be a MultipleAlignment object or derivative". I tend to use the latter in my more recent packages. (I also tend to omit the quotes around the class name in error messages.)
• The curly brackets are not needed in these simple and very common if () stop() statements. Just:

  if (!is(x, "MultipleAlignment"))
      stop("'x' must be a MultipleAlignment object or derivative")
  

Thanks again,
H.

[ahl27]

Thanks! Fixed these points.

Good point regarding the error message and on inherits. I think is is slightly faster, so at the very least that's a nice benefit.

The curly brackets are not needed in these simple and very common if () stop() statements

This makes sense. I've seen people that like the curly braces always being there and people that don't. I usually err on the side of inclusion, but I do prefer this style.

[hpages]

Thanks @ahl27