[issue28933] AC: Accept None as a Py_ssize_t default value

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> rejected
stage:  -> resolved

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Changes by Julien Palard :


--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Larry Hastings]

Larry Hastings added the comment:

Since this is the first time anybody has needed it, I suggest the latter.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

Hi Larry,

Do you mean a new converter in clinic.py like Nullable_Py_ssizze_t, or a 
converter that I copy/paste every time I need it like 
http://bugs.python.org/review/28754/patch/19417/76440 ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Larry Hastings]

Larry Hastings added the comment:

I don't want this change committed to CPython, you can do what you need with a 
converter so do that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

First, it looks weird if the default value affects accepted types. Second, how 
many use cases for your converter besides the bisect module?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

> for just one use case

I don't think that using None in a default argument is "one use case", nor a 
"special case" it's more like a "widly used pattern" that I'd like to make 
simple to implement (also see http://bugs.python.org/issue28754#msg282891).

I'm not sure you're against my idea of accepting None like:

something: Py_ssize_t(c_default="-1") = None

Or you're against my implementation, which I can understand, I'm not a fan of 
modifying getargs.c for this, it may be doable by implementing a new converter 
in clinic.py like "Py_ssize_t_optional", and it may not require more work than 
modifying getargs.c.

But AC is already defining 4 different "default values", counting the "= value" 
from the AC DSL, I'm not sure that adding the "optional" string somewhere make 
any of this clearer, typically what about the semantic difference between:

something: Py_ssize_t_optional()

and

something: Py_ssize_t() = None

So my original idea was to accept "= None" indistinctly of the used type, I 
mean allowing:

something: Py_ssize_t(c_default=…) = None
something: float(c_default=…) = None
something: double(c_default=…) = None
…
…

Which is, I think, one of the clearest way to express "It's an optional 
parameter, on the Python side the default value is None, but c-side the default 
value is of the right type an is …".

In other words, I'm proposing to optionally extend the "c_default" usage from 
"not given" to "not given or given None".

I failed to implement if for any types, mainly because AC is delegating to 
getargs for Py_ssize_t, so I though starting with my specific need (Py_ssize_t 
for issue28754) was a good start and POC.

Another solution may be to change the Py_ssize_t_converter to drop the 
format_unit, allowing us to implement the converter in clinic instead of 
getargs.c. It's opening a long term door of completly dropping non-"O" format 
units. I mean, if we drop every non-"O" format units in clinic we'll be able to 
use a faster implementation of _PyArg_ParseStack, one not parsing a 
mini-arg-definition-language at runtime (the "OO|nO&:").

I'm willing to implement something clean for this but please be more precise in 
your feedback.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Agreed with Larry. "Special cases aren't special enough to break the rules." 
General converter shouldn't be changed for just one use case. Write your own 
special converter. Or just use the "O" converter and manually convert Python 
object to C value. That can be simpler and more readable.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

Hi Larry,

Did you take more time to review this patch and my last comments? I don't think 
it that awful as it does _not_ apply until explicitly asked for, but I'm open 
to discuss it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

> You propose an automatic conversion of "None" into "-1"?  That's awful.  
> Please don't commit that patch to CPython.

Not really, I propose a way to do it with AC when needed. And the AC semantics 
introduced are not "Automatic conversion of None into -1" but "automatic "don't 
touch the C default" when the python default is given", so it's reusable for 
other values:

something: Py_ssize_t(c_default="-1") = None
something: Py_ssize_t(c_default="0") = None
something: Py_ssize_t(c_default="42") = None
…

And this semantic can be extended to other types too if needed:

something: a_type(c_default="a_c_side_default_value") = None

To get "a_c_side_default_value" when None is given.

I however did not introduced a way to keep the C default value by using 
something else than None in the Python side:

 - I'm not sure this is usefull to allow it but it can still be implemented if 
needed
 - Limiting to None permits to keep a type check so ``something: 
Py_ssize_t(c_default="-1") = "bar"`` will still fail at clinic.py runtime.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

> It looks like you did it with a converter for 28754.  That's okay.  But not 
> in the default implementation.

It's not by default, we have to declare "… = None" in the AC declaration, which 
was an error before my patch (incompatible types…).

Before:

something: Py_ssize_t(c_default="-1") = None

> Py_ssize_t_converter: default value None for field something is not of 
type int

After:

something: Py_ssize_t(c_default="-1") = None

> If the None (default) value is provided, the c_default is untouched.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Larry Hastings]

Larry Hastings added the comment:

It looks like you did it with a converter for 28754.  That's okay.  But not in 
the default implementation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Larry Hastings]

Larry Hastings added the comment:

You propose an automatic conversion of "None" into "-1"?  That's awful.  Please 
don't commit that patch to CPython.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Julien Palard added the comment:

Proposed a patch, but I'm not a huge fan of modifying getargs.c. If it's 
accepted, I'll obviously need to write tests before this is merged.

--
keywords: +patch
Added file: http://bugs.python.org/file45838/issue28933.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28933] AC: Accept None as a Py_ssize_t default value

[Julien Palard]

Changes by Julien Palard :


--
title: AC: Accept None as a default value for any type -> AC: Accept None as a 
Py_ssize_t default value

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com