[MERGE] working tree cleanups

John Arbash Meinel john at arbash-meinel.com
Sat Jul 15 16:15:10 BST 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Fri, 2006-07-14 at 09:19 -0500, John Arbash Meinel wrote:
> 
>>> === modified file 'bzrlib/workingtree.py'
>>> --- bzrlib/workingtree.py	2006-07-03 18:27:35 +0000
>>> +++ bzrlib/workingtree.py	2006-07-13 10:45:47 +0000
>>> @@ -53,6 +53,8 @@
>>>  
>>>  from bzrlib import bzrdir, errors, osutils, urlutils
>>>  from bzrlib.atomicfile import AtomicFile
>>> +import bzrlib.branch
>>> +import bzrlib.bzrdir as bzrdir
>> ^^^ we already import bzrdir
>>
>> from bzrlib import bzrdir
>>
>> and I think that is the preferred form now. I know it will work much
>> better with demandload. And I believe pyflakes complains less.
> 
> AIUI from bzrlib import bzrdir will not work unless bzrdir is already
> imported by the codebase - bzrdir is an attribute of bzrlib only after
> the import of the children is complete. I may be wrong here - its
> possibly worth a quick test. Anyway, I'm happy to remove the 'from
> bzrlib import bzrdir' instance, which is not in alphabetical order :).
> 

No, I already tested that when we were proposing to change it. It does
the right thing *unless* you have a local variable 'bzrdir' inside the
module.

% mkdir foo
% mkdir foo/bar
% echo "print 'import foo'" > foo/__init__.py
% echo "print 'import bar'" > foo/bar/__init__.py

% python -c 'from foo import bar; print bar'
import foo
import bar
<module 'foo.bar' from 'foo/bar/__init__.pyc'>
% echo "bar = 'hello'" >> foo/__init__.py
% python -c 'from foo import bar; print bar'
import foo
hello

I looked into this quite a bit. and 'from bzrlib import bzrdir' works,
and works better with demandload. I really prefer it if we switch.

I'm not sure what you mean by 'it isn't in alphabetical order'. I would
say a plain 'from bzrlib' comes before any other imports.

Maybe you would prefer:

from bzrlib import (
		    bzrdir,
		    errors,
		    osutils,
		    urlutils,
		   )

But I do think we want a single 'from bzrlib' rather than lots of:

from bzrlib import bzrdir
from bzrlib import errors
from bzrlib import ...


...

> It sure was returning something. This is the old code:
>             raise NotImplementedError
>         if not isinstance(a_bzrdir.transport, LocalTransport):
>             raise errors.NotLocalUrl(a_bzrdir.transport.base)
>         control_files = self._open_control_files(a_bzrdir)
>         return WorkingTree3(a_bzrdir.root_transport.local_abspath('.'),
>                             _internal=True,
>                             _format=self,
> 
> 
> I'll hold of merging this till we have a clear consensus on the import
> thing.
> 
> -Rob

Sure, I was just misreading the diff. I'm okay with the change. Just the
import thing now.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD4DBQFEuQZ+JdeBCYSNAAMRAqHvAJ9wm4GdhxQTMT2l6sO54GoAL52rUACYlgrd
IVj0knf+tm27oWh+CHGB0g==
=MmxL
-----END PGP SIGNATURE-----




More information about the bazaar mailing list