<br><br><div class="gmail_quote">On Wed, May 2, 2012 at 3:28 PM, Eric V. Smith <span dir="ltr"><<a href="mailto:eric@trueblade.com" target="_blank">eric@trueblade.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 05/02/2012 02:53 PM, Brett Cannon wrote:<br>
<br>
> You actually don't need to explicitly type-check and instead can rely on<br>
> duck typing::<br>
><br>
> if loader is None: continue<br>
> elif hasattr(loader, 'load_module'): return loader<br>
> else:<br>
> namespace.append(loader)<br>
> continue<br>
<br>
</div>While I agree that this accomplishes the job, I don't think it's any<br>
more readable than the existing code:<br>
<br>
if isinstance(loader, str):<br>
namespace.append(loader)<br>
elif loader:<br>
return loader<br>
<br>
(with the case of None causing the code to loop)<br>
<br>
But I'm open to changing it.<br>
<br></blockquote><div><br></div><div>I honestly don't care. I just wanted to point out to Martin that if he wanted a more interface check over type check it's totally doable.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
As to the three return types: Given that find_module() has all of the<br>
information, I don't think it makes sense to add another method. And for<br>
backward compatibility, we need to keep the {None, loader} return types.<br>
If you agree that adding another method is wasteful (it will have to do<br>
most of the same work as find_module(), or cache its result), then I<br>
think adding a str return type makes the most sense.<br>
<br>
I can't foresee this ever causing an actual problem. No one is going to<br>
subclass a loader from str (famous last words, I know!).</blockquote><div><br></div><div>Just as I know PJE is going to point out that your loader test won't work if a loader happens to be false and thus you should do an explicit ``is not None`` check. </div>
</div>