[Biopython-dev] [Bug 2754] Bio.PDB: Parse warnings should print to stderr, not stdout

[bugzilla-daemon at portal.open-bio.org]

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





------- Comment #9 from eric.talevich at gmail.com  2009-02-14 17:06 EST -------
(In reply to comment #8)

Yes, something must be done with test_PDB.py, because I don't think
warnings.warn can be made to play nice with that print-and-compare test -- or
any print-and-compare, since the warning messages contain extra
environment-specific information.

The test suite I'm picturing looks like this:
- Load the PDB file with permissive=0, verify the first PDBException.
- Add a warning filter to silence the first message, retry loading, verify the
next Exception.
- Repeat for all the expected errors in the PDB file.
- Silence warnings and load the PDB file with permissive=1; continue the usual
print-and-compare tests.

I think doctest can be coaxed into ignoring part of an output message with
ellipses, and unittest might have an assertion for error messages or we could
just catch the exception and check the message directly. So: either way,
test_PDB.py gets a rewrite, and the example PDB file can stay the way it is.

> For example, running the test from the command line the first message is:
> PDBConstructionException: Atom N defined twice in residue <Residue ARG het= 
> resseq=2 icode= > at line 19.
> Exception ignored. 
> 
> Is that correct or desired output? 

Yes, but warnings.warn prepends the absolute file path and the line number
where the warning was raised (there's an option to make it look deeper in the
stack, for catch-and-release cases like this one), so even if sys.stdout is
assigned to sys.stderr, the text doesn't match exactly and the test fails. The
important thing is that a PDBConstructionError is raised, and to be precise,
that the message contains "defined twice", as I understand it.

> The actual error is in my mind irrelevant although I do wonder why a special
> exception is used.

Two advantages for the user:
1. Tracebacks make it clear that there was a problem parsing the PDB file.
Otherwise, it's a little unclear whether there's a problem in the user's code,
a real bug in Biopython, or something wrong with the PDB file itself.
2. User code can catch a PDBConstructionException specifically and let other
exceptions fall through, e.g. an IOError which could require different
handling.

> (In reply to comment #6)
> There are a few cases of this so I think a separate bug should be filed. But
> cleaning these up would be appreciated, at least by me.

Cases of file-specific error handling, or sys.stderr/stdout abuse? Both sound
like good cleanup tasks.

In the case of __debug__ protection, it looks like normally Python executes
with __debug__==True except when run with -O. Like turning off assertions, you
know. Given that, and the simplicity of turning off warnings globally in user
code (import warnings; warnings.simplefilter('ignore')), I think it's safe to
remove these checks and just issue the warnings directly.

For the other stunt in PDBParser, that seems like it deserves a separate patch
at the very least, so I'm not going to attempt to resolve it in this bug unless
it's breaking something else.


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