Some more feedback: > This proposal builds directly upon concepts originally introduced > in :pep:`550`.
The phrase "builds upon" typically implies that the other resource must be read and understood first. I don't think that we should require PEP 550 for understanding of PEP 567. Maybe "This proposal is a simplified version of :pep:`550`." ? > The notion of "current value" deserves special consideration: > different asynchronous tasks that exist and execute concurrently > may have different values. This idea is well-known from thread-local > storage but in this case the locality of the value is not always > necessarily to a thread. Instead, there is the notion of the > "current ``Context``" which is stored in thread-local storage, and > is accessed via ``contextvars.get_context()`` function. > Manipulation of the current ``Context`` is the responsibility of the > task framework, e.g. asyncio. This begs two (related) questions: - If it's stored in TLS, why isn't it equivalent to TLS? - If it's read-only (as mentioned in the next paragraph) how can the framework modify it? I realize the answers are clear, but at this point in the exposition you haven't given the reader enough information to answer them, so this paragraph may confuse readers. > Specification > ============= > [points 1, 2, 3] Shouldn't this also list Token? (It must be a class defined here so users can declare the type of variables/arguments in their code representing these tokens.) > The ``ContextVar`` class has the following constructor signature: > ``ContextVar(name, *, default=no_default)``. I think a word or two about the provenance of `no_default` would be good. (I think it's an internal singleton right?) Ditto for NO_DEFAULT in the C implementation sketch. > class Task: > def __init__(self, coro): Do we need a keyword arg 'context=None' here too? (I'm not sure what would be the use case, but somehow it stands out in comparison to call_later() etc.) > CPython C API > ------------- > TBD Yeah, what about it? :-) > The internal immutable dictionary for ``Context`` is implemented > using Hash Array Mapped Tries (HAMT). They allow for O(log N) ``set`` > operation, and for O(1) ``get_context()`` function. [...] I wonder if we can keep the HAMT out of the discussion at this point. I have nothing against it, but given that you already say you're leaving out optimizations and nothing in the pseudo code given here depends on them I wonder if they shouldn't be mentioned later. (Also the appendix with the perf analysis is the one thing that I think we can safely leave out, just reference PEP 550 for this.) > class _ContextData Since this isn't a real class anyway I think the __mapping attribute might as well be named _mapping. Ditto for other __variables later. -- --Guido van Rossum (python.org/~guido)
_______________________________________________ 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
