Hi Florent,

Sorry for the delay.  I did try the patch out shortly after you contributed it, but it caused the functional to fail.  I was able to fix the issue and allow the existing tests to start passing, but I've been bogged down lately and haven't been able to perform a more thorough review of the code. If you could provide tests with files (e.g. for the tools affected) that test the new functionality, that would be a great help. 

The use of partition removes python compatibility for <2.5, although this is a lesser/non-concern.

Also, I'm not entirely sold on having the "Identifier line" being parsed as  "identifier" + <space> + "description" instead a single identifier line. This would mean that identifiers could not themselves contain spaces, but "There is no standardization for identifiers" (so they could technically have spaces?). Could two different reads be identified as "Read A" and "Read B", but then would no longer be uniquely identifiable as each would then be identified as "Read".  If this added functionalilty were introduced as optional behavior (e.g. a user needs to click a checkbox on the tools to apply the id line splitting), these concerns can be mitigated. 


Peter, Florent, anyone else: I'd be very interested to hear your thoughts on the above, particularly in respect to know real-world data. For now, lets discount SRA data from this discussion.


Thanks,

Dan



On Oct 19, 2011, at 5:00 AM, Peter Cock wrote:

On Wed, Oct 19, 2011 at 4:53 AM, Florent Angly <florent.angly@gmail.com> wrote:

I have had the chance to try the patch on several datasets and it looks good
:)
I reiterate my suggestion to pull the patch in galaxy-central.
Best,
Florent

It looks sensible, although I would add a comment to the join method
to say the description from read1 is taken (if the reads differ in their
descriptions). Mind you, the whole module seems to lack docstrings ;)

Are there any unit tests (not that Galaxy seems to insist on them)?

Peter
___________________________________________________________
The Galaxy User list should be used for the discussion of
Galaxy analysis and other features on the public server
at usegalaxy.org.  Please keep all replies on the list by
using "reply all" in your mail client.  For discussion of
local Galaxy instances and the Galaxy source code, please
use the Galaxy Development list:

 http://lists.bx.psu.edu/listinfo/galaxy-dev

To manage your subscriptions to this and other Galaxy lists,
please use the interface at:

 http://lists.bx.psu.edu/