Thanks for taking the time for the review, Barry.  Again, sorry I broke the review link.  It shouldn't be a problem any more.

> Barry A. Warsaw <barry@python.org> added the comment:
>
> I'm inclined to go with the as_simple_namespace patch.  As you say,
> the pro are that this is a much better fit for this use case, while
> the con is that this does kind of sneak in a new type.  Given that
> the type is not exposed in the API, that doesn't bother me.  One
> other possibility would be to move all that code to sysmodule.c as
> static functions, and then it definitely won't be available outside
> sys.
>
> OTOH, I do think this could eventually be a useful type to expose,
> however the current behavior of its repr filtering out _names would
> have to be removed in that case.  However, that's a useful filter for
> sys.implementation and it doesn't bother me, since you can always
> repr sys.implementation.__dict__ to get the whole thing.
>
> On balance, my recommendation would be to keep it the way you have
> it, but perhaps mention this on python-dev, and watch the technicolor
> bikeshed go psychedelic.

:)  I'll bring it up on python-dev.

>
> A few other comments as we discussed in irc:
>
> dir(sys.implementation) should return something useful, not traceback

Not sure what I was doing to block the lookup from object, but it's working now.

So...Fixed.

>
> There are a few PEP 7 violations, e.g. brace placements that should
> be fixed

Done.

>
> I was looking at _namespace_init() and a few things bothered me.  I
> thought this might be superfluous and that you'd be able to just
> inline the PyDict_Update() calls, but now I'm not so sure.  AFAICT,
> Python's documentation is silent on whether *args and *kwds in a
> tp_init function can be NULL or not, and I definitely see cases where
> similar checks are made in other types.  So I think with that in
> mind, you must assume they *can* be NULL.  But then I'm not sure the
> assertions are useful or correct.

Yeah, they're mostly an artifact of copying code.  However, I'm glad they triggered your explanation. ;)

> Take this scenario:
>
> namespace_init() gets args==NULL.  Your assertion doesn't trigger,
> but PyObject_Size(NULL) sets an exception and returns -1.  Your
> conditional doesn't check for an error condition in PyObject_Size()
> so you'll incorrectly swallow (temporarily) that exception.  At the
> very least, you need to check for PyObject_Size() < 0.  Don't forget
> too that those assertions can get compiled away.
>
> I think I'd rather see a PyTuple_Size(args) conditional here for the
> args parameter.  If it's not a tuple, you'll get an exception set, so
> check for size < 0.  If size > 0, then you can set the TypeError and
> return -1 explicitly.

Pretty sure I understand.  Let me know if my new patch says otherwise.  <wink>

So...Done.

>
> Similarly, just call PyDict_Update(kwds) and return its value.  If
> kwds is neither a dict nor has a .keys() method (including if its
> NULL), an exception will be set so everything should work correctly
> (see _PyObject_CallMethodId() in abstract.c, which is what
> PyDict_Update() boils down to).

Done.  This was a case of prematurely optimizing.

>
> At least, I'm nearly certain that's safe :)
>
> Moving on...
>
> namespace_repr() also has some PEP 7 violations (brace position,
> extra blank lines).  Please fix these.

Done.

>
> Is it ever possible to get into namespace_repr() with ns->ns_dict
> being NULL?  I think it's impossible.  You might rewrite the if
> (d == NULL) check with an assertion.

Done.

>
> I think you're leaking the PyList_New(0) that you put in `pairs`
> after you PyUnicode_Join() it.  PyUnicode_Join() doesn't decref its
> second argument and your losing the original referenced object when
> you assign `pairs` to its result.

Good Catch.  Fixed.

>
> I find the mix of inline error returns and `goto error` handling in
> namespace_repr() somewhat confusing.  I wonder if it would be more
> readable (and thus auditable) if there was consistent use of `goto
> error` with more XDECREFs?  If you feel like it, please try that out.

I've given it a go.  :)

>
> That's it for now.  Please submit another patch for the
> as_simple_namespace case and I'll take another look.  Also, if you
> send a message to python-dev about the introduction of the namespace
> object, I'll follow up with my support of that option.
>
> Thanks, this is looking great!

Thanks again for all your support through this process!