Nice working staying on top of this! Keeping up with discussion is arguably much harder than actually writing the PEP. :) I have some comments in-line below.
-eric On Fri, Sep 1, 2017 at 5:02 PM, Yury Selivanov <yselivanov...@gmail.com> wrote: > [snip] > > Abstract > ======== > > [snip] > > Rationale > ========= > > [snip] > > Goals > ===== > I still think that the Abstract, Rationale, and Goals sections should be clear that a major component of this proposal is lookup via chained contexts. Without such clarity it may not be apparent that chained lookup is not strictly necessary to achieve the stated goals (i.e. an async-compatible TLS replacement). This matters because the chaining introduces extra non-trivial complexity. > The goal of this PEP is to provide a more reliable > ``threading.local()`` alternative, which: > > [snip] > > * implements TLS-like semantics for synchronous code, so that > users like ``decimal`` and ``numpy`` can switch to the new > mechanism with minimal risk of breaking backwards compatibility; > > [snip] > > Replication of threading.local() interface > ------------------------------------------ > > Choosing the ``threading.local()``-like interface for context > variables was considered and rejected for the following reasons: > > * A survery of the standard library and Django has shown that the > vast majority of ``threading.local()`` uses involve a single > attribute, which indicates that the namespace approach is not > as helpful in the field. > > * Using ``__getattr__()`` instead of ``.get()`` for value lookup > does not provide any way to specify the depth of the lookup > (i.e. search only the top logical context). > > * Single-value ``ContextVar`` is easier to reason about in terms > of visibility. Suppose ``ContextVar()`` is a namespace, > and the consider the following:: > > ns = contextvars.ContextVar('ns') > > def gen(): > ns.a = 2 > yield > assert ns.b == 'bar' # ?? > > def main(): > ns.a = 1 > ns.b = 'foo' > g = gen() > next(g) > # should not see the ns.a modification in gen() > assert ns.a == 1 > # but should gen() see the ns.b modification made here? > ns.b = 'bar' > yield > > The above example demonstrates that reasoning about the visibility > of different attributes of the same context var is not trivial. > > * Single-value ``ContextVar`` allows straightforward implementation > of the lookup cache; > > * Single-value ``ContextVar`` interface allows the C-API to be > simple and essentially the same as the Python API. > > See also the mailing list discussion: [26]_, [27]_. > > [snip] On the one hand the first three sections imply that the PEP is intended as a replacement for the current TLS mechanism; threading.local. On the other hand, the PEP (and related discussion) clearly says that the feature works differently than threading.local and hence is not a (drop-in) replacement. I'd prefer it to be a drop-in replacement but recognize I'm on the losing side of that argument. :P Regardless, having a consistent message in the PEP would help folks looking to switch over. Speaking of which, I have plans for the near-to-middle future that involve making use of the PEP 550 functionality in a way that is quite similar to decimal. However, it sounds like the implementation of such (namespace) contexts under PEP 550 is much more complex than it is with threading.local (where subclassing made it easy). It would be helpful to have some direction in the PEP on how to port to PEP 550 from threading.local. It would be even better if the PEP included the addition of a contextlib.Context or contextvars.Context class (or NamespaceContext or ContextNamespace or ...). :) However, I recognize that may be out of scope for this PEP. _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com