[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 #8 from lpritc at scri.sari.ac.uk  2008-12-15 05:41 EST -------
(In reply to comment #0)
> 1) Why are there two functions to output a diagram than just one generic
> function? In particular, why not just pass a filename or not? 

When I wrote the libraries originally, I had one main use in mind: production
of publication-quality images in vector format.  Later on I decided that I
needed streaming output for web display, and then bolted on the
write_to_string() to look like the ReportLab interface, for consistency. 
That's why there are two methods: the write() method produces
publication-quality (and bitmaps, if you ask), and the write_to_string() method
produces the streaming output.

It should be possible to make write() do both jobs, so long as the intention is
declared in the argument list.  It might be nice to just be able to specify a
stream or handle, rather than the filename.  Both of these would be an API
change.

> 2) I find the functions write() and write_to_string() just plain ugly. 
> You define a local dictionary of modules every time these functions are called.

That dictionary could be placed at the head of the script to be defined on
import.  But I think it's more explicit what's going on to have it in the
method itself - the dictionary has restricted scope, and is garbage-collected
after the function call.  Also, I don't understand your nested list proposal:
distribution dictionaries are not that uncommon.

> 4) I do not know the policy on module imports, but this line is only required
> for write() and write_to_string():
> from reportlab.graphics import renderPS, renderPDF, renderSVG, renderPM
> Also renderPM is an addon.

Apologies for repeating myself earlier about this one - Bugzilla was being
flaky - but renderPM is now part of ReportLab 2.2.  Whether we should continue
to support/cater for installations of 1.21 without the add-ons is another
question, I think.


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