[BioPython] performance problem in ParserSupport.EventGenerator._get_set_flags

Brad Chapman chapmanb at uga.edu
Thu Jun 12 15:53:13 EDT 2003


Hi Andreas;

[EventGenerator speedup attempts]
> > Okay, so that means it didn't break anything new, right? That's a
> > good thing.
> 
> probably. But what confuses me is that one of the failing tests is
> GenBank. (ValueError: Unexpected function name: CONTIGjoin)
> It's also failing for cvs version. 

This is with the patch, right? I just checked out a clean CVS and
test_GenBank passes fine for me without any changes. We definitely
need to have it working at the start :-).

After the patch I do get the error you mention, which means I
probably didn't do something right. I'll need to look at it more
this weekend to see what I messed up.

> But would the new code be wrong, I think some other tests should fail,
> because ParserSupport is used all over the place.

True, but EventGenerator is only used in a few places. GenBank,
ECell, Emboss/Primer, IntelliGenetics, and KEGG look like the
places, from a quick grep.

> I have some ideas for this. First I want to know if I can make following
> assumptions:
> 
> 1. the values in ParserSupport.EventGenerator.flags are allways 0 or 1

Yup.

> 2. the keys are allways same as ParserSupport.EventGenerator.interest_tags

Yup.

> 3. Nobody outside ParserSupport.EventGenerator uses these flags
> (Probably hardest to confirm.)

I quickly grepped through and didn't see anything. We should
probably rename self.flags to self._flags to reflect that you
shouldn't be using it from external classes if we get rid of
_get_set_flags.

> Then we could get rid of _get_set_flags and just put only the set flags
> into the dict.

That sounds like it should work. If you send a patch I'm happy to
try and get it in.

> This would give another 10% performance boost for my test prog. 

10 percent is good. I'm all about it.
Brad


More information about the BioPython mailing list