[MERGE] bzrlib.osutils fixes

[Andrew Bennetts]

John Arbash Meinel wrote:
[...]
>
> 1) 'directory' is 'directory'. String constants are automatically
> interned if the "look like python variables" (according to spiv).

For CPython, anyway.  I don't think it's part of the language definition.

Also, note that interning only speeds up comparisons of equal strings.  Two
unequal strings will take the exact same code path to compare regardless of
whether they are interned or not.

> 4) If you have _directory_kind, it will be a variable lookup. (possibly
> in locals, frequently in globals). I don't know exactly how python
> handles constants, but "if x is 'directory'" could certainly be faster
> than "if x is _directory_kind".

It is slightly faster, but I suspect this is so slight that we don't care:

$ python -m timeit -s 'f = "foo"; b = "bar"' 'f is not b'
10000000 loops, best of 3: 0.137 usec per loop
$ python -m timeit -s 'f = "foo"' "f is not 'bar'"
10000000 loops, best of 3: 0.135 usec per loop
$ python -m timeit "'foo' is not 'bar'"
10000000 loops, best of 3: 0.132 usec per loop

> 5) 'if x == _directory_kind' is a bit less transparent than "if x ==
> 'directory'". So there is a readability factor.

Yes and no.  _directory_kind in some ways conveys more information to the reader
than a bare literal.  That said it is certainly uglier to look at.

> 6) If we do decide to go with _foo_kind, then we should be consistent,
> and have _file_kind, _directory_kind, _symlink_kind, etc.

Agreed, or at least if not there ought to be a clear explanation in the code to
say why _directory_kind is special.  Having just _directory_kind is surprising.

-Andrew.