<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 1, 2017 at 8:29 PM, Eric Snow <span dir="ltr"><<a href="mailto:ericsnowcurrently@gmail.com" target="_blank">ericsnowcurrently@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Nice working staying on top of this!  Keeping up with discussion is<br>
arguably much harder than actually writing the PEP. :)  I have some<br>
comments in-line below.<br>
<br>
-eric<br>
<br>
<br>
On Fri, Sep 1, 2017 at 5:02 PM, Yury Selivanov <<a href="mailto:yselivanov.ml@gmail.com">yselivanov.ml@gmail.com</a>> wrote:<br>
> [snip]<br>
><br>
> Abstract<br>
> ========<br>
><br>
> [snip]<br>
><br>
> Rationale<br>
> =========<br>
><br>
> [snip]<br>
><br>
> Goals<br>
> =====<br>
><br>
<br>
I still think that the Abstract, Rationale, and Goals sections should<br>
be clear that a major component of this proposal is lookup via chained<br>
contexts.  Without such clarity it may not be apparent that chained<br>
lookup is not strictly necessary to achieve the stated goals (i.e. an<br>
async-compatible TLS replacement).  This matters because the chaining<br>
introduces extra non-trivial complexity.<br></blockquote><div><br></div><div>In defense of the PEP, the Goals section clearly states it aims to provide an alternative, not a plug-in API equivalent.</div><div><br></div><div>There's also been some discussion (I hope it wasn't off-list) on how to implement threading.local on top of this proposal.</div><div><br></div><div>That said, I agree it would be nice if the chained lookup was mentioned in the abstract too, because it's a pretty essential part of the proposal (it determines the semantics of ContextVar). (OTOH the HAMT implementation is less essential, there's another way to do the chained lookup.)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">> [snip]<br>
<br>
On the one hand the first three sections imply that the PEP is<br>
intended as a replacement for the current TLS mechanism;<br>
threading.local.  On the other hand, the PEP (and related discussion)<br>
clearly says that the feature works differently than threading.local<br>
and hence is not a (drop-in) replacement.  I'd prefer it to be a<br>
drop-in replacement but recognize I'm on the losing side of that<br>
argument. :P  Regardless, having a consistent message in the PEP would<br>
help folks looking to switch over.<br>
<br>
Speaking of which, I have plans for the near-to-middle future that<br>
involve making use of the PEP 550 functionality in a way that is quite<br>
similar to decimal.  However, it sounds like the implementation of<br>
such (namespace) contexts under PEP 550 is much more complex than it<br>
is with threading.local (where subclassing made it easy).  It would be<br>
helpful to have some direction in the PEP on how to port to PEP 550<br>
from threading.local.  It would be even better if the PEP included the<br>
addition of a contextlib.Context or contextvars.Context class (or<br>
NamespaceContext or ContextNamespace or ...). :)  However, I recognize<br>
that may be out of scope for this PEP.<br clear="all"></blockquote><div><br></div><div>If you look at what decimal does, it only ever stores a single value in its threading.local instance -- __decimal_context__. (And part of the reason is that the C version has to work with a different API for TLS, which really does only store a single value per key.)<br></div></div><br>-- <br><div class="gmail_signature">--Guido van Rossum (<a href="http://python.org/%7Eguido" target="_blank">python.org/~guido</a>)</div>
</div></div>