[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]

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





------- Comment #15 from biopython-bugzilla at maubp.freeserve.co.uk  2008-12-23 06:12 EST -------
(In reply to comment #14)
> (In reply to comment #12)
> > OK, so its less useful than I had expected.  Rending bitmaps to strings so
> > they can be inserted into a database as blobs is one potential use-case.
> > Also for a web-service where you expect the user to save and print the
> > naked image (unusual, and probably software dependent on how the DPI is
> > treated).
> 
> Surely it is important because a user can write to a string and then save the
> string to a file rather than using write() a second time. 

I was talking about write to string with a DPI not being so useful.

Using write to string is VERY useful, particularly for a webserver (which is
why Leighton added it, and how I have used it).  Setting the DPI isn't
important for using images in webpages - HTML and CSS provide lots of ways to
control the displayed and printed size.  Even if the browser is pointed
directly at the image (and not as part of a webpage) and you then print it, the
browser may ignore the DPI setting (probably browser specific).  i.e. The DPI
will only matter if the user saves the image and opens it in DPI aware
software.

(In reply to comment #14)
> (In reply to comment #12)
> > However, I'm not so keen on this style personally - the return value is
> > radically different depending on the arguments (nothing, or a string of
> > data).
> > 
> > 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.
> 
> I agree and I am not very concerned about backwards compatibility since this
> is a very new function to Biopython. I think that is what is almost what
> write_to_string() does and python functions are very big. But this is not my
> code so please do as you want here.

GenomeDiagram is new to Biopython, but has been available independently for
many years.  There will be some existing users (not just me and Leighton), and
the less they have to change to switch their code from using standalone
GenomeDiagram to the one within Biopython the better (the import lines have to
change for example).  We do need to think about backwards compatibility a bit.

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().

(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.

(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.

Peter


-- 
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.