GeezerVison

On Thu, Dec 28, 2017 at 4:51 AM, Victor Stinner
<victor.stinner at gmail.com> wrote:
> Hi,
>
> I like the new version of the PEP using "read only mapping" and
> copy_context(). It's easier to understand.

Thanks, Victor!

>
> I'm ok with seeing a context as a mapping, but I am confused about a context
> variable considered as a mapping item. I still see a context variable as a
> variable, so something which has a value or not. I just propose to rename
> the default parameter of the ContextVar constructor.

As Nathaniel already explained, a 'default' for ContextVars is
literally a default -- default value returned when a ContextVar hasn't
been assigned a value in a context.  So my opinion on this is that
'default' is the less ambiguous name here.

[..]
>
> * a read-only attribute ``Token.old_value`` set to the value the
>   variable had before the ``set()`` call, or to ``Token.MISSING``
>   if the variable wasn't set before.
>
>
> Hum, I also suggest to rename Token.MISSING to Token.NOT_SET. It would be
> more conistent with the last sentence.

I like MISSING more than NOT_SET, but this is very subjective, of
course.  If Guido wants to rename it I rename it.


> C API
> -----
>
>
> Would it be possible to make this API private?

We want _decimal and numpy to use the new API, and they will call
ContextVar.get() on basically all operations, so it needs to be as
fast as possible.  asyncio/uvloop also want the fastest copy_context()
and Context.run() possible, as they use them for *every* callback.  So
I think it's OK for us to add new C APIs here.


>
> 2. ``int PyContextVar_Get(PyContextVar *, PyObject *default_value,
> PyObject **value)``:
>     (...)  ``value`` is always a borrowed
>    reference.
>
>
> I'm not sure that it's a good idea to add a new public C function which
> returns a borrowed reference. I would prefer to only use (regular) strong
> references in the public API.

Sure, I'll change it.

Yury