[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


bsouthey at gmail.com changed:

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




------- Comment #17 from bsouthey at gmail.com  2009-01-05 11:30 EST -------
I do not consider this bug completely fixed for multiple reasons of which my
patch addressed some of these prior to the creation of the _write function. I
do like where _write is heading as it is making cleaner and more understandable
code.

1) I do not understand the need for the dictionary of modules 'formatdict' in
_write as it creates unnecessary inefficient code. The options need to be part
of the check for the type of output.

2) There is no indication that the output for write and write_to_string only
accepts uppercase. Note the _write function states this but a user will not see
these. I do not understand why lowercase is unacceptable. 

3) The check for renderPM at start is really redundant because _write checks
for it (well sort of). It is also an unnecessary delay if renderPM is not used.
If you really must use the dictionary (which I really do not like) I would
suggest something like:
formatdict = {'PS': renderPS, 'PDF': renderPDF,'SVG': renderSVG}
try:
    from reportlab.graphics import renderPM
    formatdict.update({'JPG': renderPM, 'BMP': renderPM, 'GIF': renderPM,
'PNG': renderPM, 'TIFF': renderPM,'TIF': renderPM})

The current code would show the correct options regardless of status
ofrenderPM. Perhaps an exception could provide a warning that renderPM is not
present.

4) There is no test for the presence of renderPM. The test function must check
for renderPM and should at least provide a warning if not present. Otherwise
this is a surprise to a user because not all options will be available.

5) The installation documentation must also indicate that renderPM is optional
and also how to install the renderPM module.


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