[Biopython-dev] [Bug 2711] GenomeDiagram.py: write() and write_to_string() are inefficient and don't check inputs

bugzilla-daemon at portal.open-bio.org bugzilla-daemon at portal.open-bio.org
Tue Dec 23 12:47:26 UTC 2008


http://bugzilla.open-bio.org/show_bug.cgi?id=2711


biopython-bugzilla at maubp.freeserve.co.uk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Comment #16 from biopython-bugzilla at maubp.freeserve.co.uk  2008-12-23 07:47 EST -------
In comment #12, I wrote:
> If we were designing this from scratch, I would have suggested one write
> function which wrote to a handle - which would let you then write to a file or
> a string (using StringIO).  On the other hand, this is perhaps a little low
> level.  We're had similar discussions regarding Bio.SeqIO in the past.

The reportlab docstrings are very unclear, however, their renderer's drawToFile
functions take either a filename OR a handle.  This works because the
underlying Canvas object can be created giving either a filename or a handle.

As a result, GenomeDiagram's write() method should accept either a filename or
a handle.  We should update the docstring to say this (perhaps even renaming
the argument?).

(In reply to comment #15)
> (1) Two functions write() and write_to_string()
> This follows the reportlab API, and they do actually return different
> encodings.

I wrote this based on something Leighton had said to me.  Going over the
reportlab code, this isn't true - reportlab's drawToString just calls
drawToFile with a cStringIO or StringIO handle.  They write identical data.

(In reply to comment #15)
> Getting back to your original points,
> 
> (1) Two functions write() and write_to_string()
> This follows the reportlab API, and they do actually return different
> encodings.  From a backwards compatibility argument they should both stay, but
> that doesn't stop us providing a unified method and deprecating 
> write_to_string().

I've filed Bug 2718 for the general issue of method naming for the Bio.Graphics
modules output functionality.

> (2) Coding style of write() and write_to_string()
> I don't have a problem with this - it works, its clear, its easily extended if
> ReportLab add more back ends.  It doesn't strike me as ugly.  Inevitably this
> is largely a matter of preference.

Leaving this as is - the code itself may end up handled via shared function for
all of Bio.Graphics via Bug 2718.

> (3) The KeyError exception with invalid arguments.
> This is fixed in CVS, for an invalid format argument you now get a ValueError
> which is standard python practice.
> 
> (4) renderPM
> Fixed in CVS, in that you can now use GenomeDiagram without ReportLab
> renderPM and have full functionality except for bitmap output.  Given we 
> don't seem to be able to assume renderPM will be installed and working, this
> seems a reasonable solution.  If you try and render a bitmap without
> renderPM, then you get a MissingExternalDependencyError exception asking you
> to install renderPM.  We will need to look into this further for the
> documentation.

Marking this bug as FIXED.


-- 
Configure bugmail: http://bugzilla.open-bio.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the Biopython-dev mailing list