On Tue, Aug 23, 2011 at 04:44:08PM -0700, Reid Price wrote: > A few notes inline, any lack of response indicates wholehearted agreement > > On Tue, Aug 23, 2011 at 2:42 PM, Ben Pfaff <b...@nicira.com> wrote: > > > If this is performance-sensitive, you may want to change some of the > > imports > > > up top from 'import ovs.db.X' to 'import ovs.db.X as X'. I've heard each > > '.' > > > requires a dereference, and is not well-optimized. This is standard > > practice > > > if there isn't any name collision to worry about. > > > I'm not sure what the actual gain is in practice, if it ever becomes an > > > issue, could try replacing ovs.db.X.Y with ovs_db_X_Y, and make a bunch > > > of globals to measure the possible gain - but as far as I know there > > > hasn't been an problem. > > > > For the code that I've written most recently, I've been trying to > > following Google's Python style guide. The way I read it, I thought > > that it forbade using "as" on imports. It's kind of a pain spelling > > out the whole names all the time, so if I'm misreading it, or if this > > style guideline is widely ignored, let me know and I'll shorten > > everything. > > > > Yeah, I think we're both coming from the same source > > http://code.google.com/p/soc/wiki/PythonStyleGuide#Module_and_package_imports > In most of the code we've been writing, we tend to use statements like > > import lib.foo.bar.baz as baz > > google's example does this in the form > > from lib.foo.bar import baz > > where baz is a module (baz.py exists) in the package. Again, the > suggestion was mostly from a performance perspective. I don't > really begrudge the chains unless they make all my code linewrap, > though I think the 'import x.y.z .. as z' version of the import avoids > masking dependencies and still allows for readable code.
There's more subtlety in the recommendations than I realized before. I'll try to go through the OVS Python code and update it at some point. > > > We should add a check here ;) > > > assert keyName not in ['ox', 'fish', 'data'] # 'octopus' is ok > > > > Ha ha, yes. We don't have any columns with those names yet :-) > > One can dream =) You just need to invent a protocol named "ox" or "fish" and your dream will come true :-) > > > Could avoid shadowing the built-in type 'tuple' by mangling to _tuple, > > > or by using: > > > def open_block((error, stream)): > > > Though that might be confused at a glance with a 2-argument function > > > def open_block(error, stream): > > > > I didn't know that worked. Neat, thank you. > > > > Should note that Peter pointed out to me that the preferred mangling > is a trailing underscore, i.e.: "tuple_" instead of "_tuple". That's what I used, in fact. I'm somewhat allergic to leading underscores since C reserves them in most contexts. > > Thanks a lot for the review! > > Thanks for taking time to look through all these and responding, I > appreciate it All of it was useful, thanks again! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
