Attached is a *preliminary* patch for Argument Clinic that adds nullable ints.  If you use the converter "int(nullable=True), then:

* The type you get back is not int but "nullable_int_t", a structure
  containing three fields: "error", "is_null", and "i".  "error" is 1 if
  there was an error in parsing, else 0.  "is_null" is 1 if None was
  passed in, else 0.  "i" is the integer value if an integer was
  passed in, else undefined.
* You may now use a default value of None.  If you do, "is_null" is 1.
* You may still use a default integer.  If you do, "i" will be set
  to that.
* If you use neither then obviously it's a required argument.


I'd appreciate just a preliminary review, saying something like
"this is a good idea, keep going," or
"you should change your approach," or
"this is a bad idea and should not be committed," or
"you look very handsome today Larry".

I don't remember who specifically needed the nullable ints, so I just added a bunch of Derby contestants.

Serhiy: I also added a fix for the bug you mentioned in #20294 after it was closed: the docstring for __new__ and __init__ methods now uses the class name instead of the name of the method.  Note that this fix doesn't matter much; once I commit a fix for #20189, that signature will always be hidden.

I should have mentioned--a sample using nullable ints is in itertoolsmodule.c.  longobject.{ch} changed to add the converters, and obviously clinic.py changed to add support for the converters.

>>> itertools.repeat.__doc__
'repeat(object, times=None)\nReturns an iterator which returns the object the specified number of times.\n\nIf times is None, returns the object endlessly.'
>>> itertools.repeat('a', times=None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __new__() argument 2 must be (unspecified), not None

Also, maybe it's a good idea to add case for None and negative times in itertools.repeat unit test. You can get inspiration from http://bugs.python.org/issue19145

The bug you saw was an easy fix; the converter function needs to return 1 (success) from the "== Py_None" branch.

And yes, more unit tests would be good, fixing the documentation would be good too.  I did say the patch was nowhere near ready.

I will assume unless I hear otherwise that I should proceed with the patch and flesh out all the different and float types.

You look very handsome today Larry :)

Regardless, I do think this is a good idea. I've converted several functions that have -1 as a default value for ints where they ideally should have None. Having None as the default will be much clearer and less error prone for users.

My only comment from reviewing the patch is that you seem to have used NULLABLE_PY_SSIZE_T_INITIALIZE by accident one time in int_converter (in clinic.py).

Other than that, as you already know, make sure to return 1 in both converters in the "if (arg == Py_None)" branches.

> I don't remember who specifically needed the nullable ints, so I just added a bunch of Derby contestants.

There are no much need in nullable ints, if they are needed, different use cases require different semantic. I don't think we can now write general converters which will cover different use cases. May be in 3.5, when behavior changes will be possible.

I know only one use case for nullable Py_ssize_t converter (and mmap_convert_ssize_t is much simpler). itertolls.repeat doesn't need it, because it shouldn't accept None.

Let defer this issue until most of code will be converted. Then we can accumulate statistic about nullable in converters.

And thank you for the fix for the __init__ methods. Could you please commit it? The I'll update patches for bz2 and lzma modules.

Looks as Vajrasky Kok has encountered with yet one bug in generated __init__ wrapper(issue20185).

Then you disagree with Guido, who says that optional integer arguments without a viable publishable default value should accept None to mean "use the default value":

https://mail.python.org/pipermail/python-dev/2014-January/131673.html
https://mail.python.org/pipermail/python-dev/2014-January/131711.html

There are a fair number of places where people are abusing positional
parameters because they can't specify a good default.  I plan to put a stop to that, but first I need to give them a viable alternative.

In the specific case of itertools.repeat, I'm going to talk to the maintainer about what he wants to do.  The current behavior is too magic for Python and iiuc Guido regards it as a bug.  My guess is a default of None, as the author was deliberately preventing you from passing in -1.  But we shall see.

I thought it was too late for 3.4, and the conversion should not change the 
behavior (we can change the behavior later in 3.5).

I guess we should ask Guido for clarification.  But I got the impression that we could *gently* tweak the signatures of functions if they were not representable in Python syntax.  He thought that giving the parameter of _sha1.sha1() a default value of b'' was fine for example, and in the case of itertools.repeat he considered the behavior so odd he was guessing it was a bug.

Actually, the documented behavior of itertools.repeat() is that the "times" argument takes a default of None.

    Equivalent to:

    def repeat(object, times=None):
        ...

http://docs.python.org/3/library/itertools.html#itertools.repeat

Found two bugs in the draft patch in Objects/longobject.c:

1) In _PyLong_NullableIntConverter, there is no index variable, so the Py_DECREF and Py_XDECREF need to be removed from the end of the function.

2) In _PyLong_NullablePy_ssize_tConverter, the index variable will point to a Python object only when both if blocks are skipped. Therefore, the Py_XDECREF after "fail:" at the end of the function needs to be removed.

With these two changes, I successfully used this patch to convert deque_init in Modules/_collectionsmodule.c, simplifying the input validation code considerably.

Some things:
- the concept of a nullable thing in Python doesn't exist; why not "optional"?
- why is there a "error" field in the new structs?
- the fact that the structs are defined in longobject.h looks bonkers
- boolean fields can be "char" instead of "int" (and moved at the end to pack the structure more efficiently)
- PyLong_AsSsize_t can't fail?
- does this change have a performance impact?

- the concept of a nullable thing in Python doesn't exist; why not "optional"?

That's not what it means. Python parameters are "optional" if they
have a default value.  These parameters are "nullable", in the sense
that they can be either of a specific type or "None".  (But "Noneable"
seemed like a bad name).  They are not necessarily optional.

My inspiration for the name was Nullable in C#:

http://msdn.microsoft.com/en-us/library/1t3y8s4s.aspx

Argument Clinic already has converters that support "nullable":
str, UNICODE, and Py_buffer.  These map to the semantics of 'z', 'S*', etc.  I guess we could change it if it were a terrible name, but it isn't.


> - why is there a "error" field in the new structs?

It seemed like a good idea at the time.  Maybe it's redundant.  Can I get back to you?


> - the fact that the structs are defined in longobject.h looks bonkers

Okay.  What would be better?  modsupport.h?


> - boolean fields can be "char" instead of "int" (and moved at the
>   end to pack the structure more efficiently)

Is Python really compiled with packed structures?  I assumed -O3 turned on dword alignment in structures.

Aligned accesses are faster, and the additional memory use would be negligible.  These will only ever be declared as stack variables in parsing functions.


> - PyLong_AsSsize_t can't fail?

Not on an object returned by PyNumber_Index().  And by the way I literally copied and pasted the code that implements 'n'--it does that too.


> - does this change have a performance impact?

Compared to just using "i", it adds the converter call and the "== Py_None" check.  The performance impact should be so small as to be difficult to measure.

> That's not what it means. Python parameters are "optional" if they
> have a default value.  These parameters are "nullable", in the sense
> that they can be either of a specific type or "None".  (But "Noneable"
> seemed like a bad name).  They are not necessarily optional.

int(or_none=True) ?

> > - the fact that the structs are defined in longobject.h looks bonkers
> 
> Okay.  What would be better?  modsupport.h?

Hmm, do we have a getargs.h ?

> > - boolean fields can be "char" instead of "int" (and moved at the
> >   end to pack the structure more efficiently)
> 
> Is Python really compiled with packed structures?

You don't understand me. If you write:
  struct X { int A; char B; char C; }
the structure will be packed *by definition* (IIRC).

> > - PyLong_AsSsize_t can't fail?
> 
> Not on an object returned by PyNumber_Index().

And what if the long is too long to fit in a Py_ssize_t?

Another bug in the patch:

In int_converter.__init__ in clinic.py, the patch adds:

if not isinstance(self.default, int):
    fail("Illegal default value for int_converter")

This fails if no default is specified, e.g. for positional only argument. The condition should be:

if not (self.default is unspecified or isinstance(self.default, int)):

Ditto for Py_ssize_t_converter.

> int(or_none=True) ?

Yes, that is a different name that seems to mean much the same thing.


> Hmm, do we have a getargs.h ?

No.  Would it help if I attached the output of "ls Include"
to this issue?


> > > - boolean fields can be "char" instead of "int" (and moved at the
> > >   end to pack the structure more efficiently)
> > 
> > Is Python really compiled with packed structures?
> 
> You don't understand me. If you write:
>   struct X { int A; char B; char C; }
> the structure will be packed *by definition* (IIRC).

I understand you fine.  That's not guaranteed by the C standard,
and I've used compilers that didn't pack structures by default.
More to the point, unaligned accesses on Intel are more expensive
than aligned accesses, and on other architectures I've written
C code using packed structures with unaligned accesses that
produced a bus error.

http://en.wikipedia.org/wiki/Data_structure_alignment#Data_structure_padding

And finally: I removed "error"--you were right, it was unnecessary
for the nullable ints.  So now it's just

  { int error; <whatever> i; }

Changing error to an char and moving it to the end would
save exactly zero bytes, because the compiler *will* align
stack variables to 4 byte boundaries.  I can give you a sample
C program if you want proof.


> > > - PyLong_AsSsize_t can't fail?
> > Not on an object returned by PyNumber_Index().
> And what if the long is too long to fit in a Py_ssize_t?

Ah.  I thought PyNumber_Index guaranteed it would fit, but
I was mistaken.  Code is fixed.


New patch posted.  I believe this fixes all the bugs cited so far.


Also, I don't know what the right thing to do about itertools.repeat
is.  The current signature is a bug, it should not behave differently
between repeat(o, -1) and repeat(o, times=-1).  If we go by the
documentation, repeat(o, None) and repeat(o, times=None) are what we
want.  But I bet code out there relies on repeat(o, times=-1), so we
may be stuck supporting that.  You should consider my signature for
itertools.repeat in these "draft" patches as a proof of concept, not
as code that should be checked in.

The issue is being discussed on #19145.

> > int(or_none=True) ?
> 
> Yes, that is a different name that seems to mean much the same thing.

and which is much more understandable by a Python developer.

> Changing error to an char and moving it to the end would
> save exactly zero bytes, because the compiler *will* align
> stack variables to 4 byte boundaries.

Except if other stack variables happen to be shorter than an int,
perhaps. But regardless, it doesn't cost anything to do so, so why not
do it?

The new patch still uses NULLABLE_PY_SSIZE_T_INITIALIZE instead of NULLABLE_INT_T_INITIALIZE in one place in int_converter.converter_init.

Fix attached.  Might as well.

> > Yes, that is a different name that seems to mean much the same
> > thing.
> and which is much more understandable by a Python developer.

Thanks for your opinion.


> > Changing error to an char and moving it to the end would
> > save exactly zero bytes, because the compiler *will* align
> > stack variables to 4 byte boundaries.
> 
> Except if other stack variables happen to be shorter than an int,
> perhaps.

Do we use a lot of those?


> But regardless, it doesn't cost anything to do so, so why not do it?

I thought I just got through explaining how it is slightly slower.

There is no benefit in changing them to "char", so why do it?

I insist that "nullable" is a very bad name. It also suggests that
somehow we'll get a NULL pointer (in C) when the parameter is absent or
None, which is not the case.

I think it's a fine name, otherwise I would not have chosen it in the first place.  Your high-spirited bikeshedding has been noted and not acted upon.  I have no plans to change the name.  Please drop it.

The only place where "nullable" appears in the doc tree is... clinic.rst (in reference to places where a C pointer can be NULL, which *is* a reasonable use of the term).

There's another problem in the patch: if you define "nullable_int_t" and "nullable_Py_ssize_t" in a public header (longobject.h), then you probably need to prefix them with "_Py" (so: "_Py_nullable_int_t" and "_Py_nullable_ssize_t", for example). Same for the #define's.

How many existing occurrences of "or_none" did you find in the CPython tree?  Perhaps we can turn our attention to other languages like SQL and C# and borrow their term for "value that is allowed to be of a specific type (or types) as well as the null value".

> How many existing occurrences of "or_none" did you find in the CPython
> tree?

If I split that into the individual words "or" and "None", quite a lot.
Looking for "or None" turns quite a bunch of matches too.

"or_none" simply applies the PEP 8 convention for variable names made of
multiple words.

Of course, you can propose another name that involves "none", too.

> Perhaps we can turn our attention to other languages like SQL and C#
> and borrow their term for "value that is allowed to be of a specific
> type (or types) as well as the null value".

It is quite well-known and obvious that Python was inspired by C# and
SQL, so why not indeed?
Perhaps you should start by asking Guido if he wants to rename None to
"NULL".

Drop.  It.

> Drop.  It.

Seriously? Why exactly did you ask for comments if your only response is
to be snarky and dismissive?

This is not your toy project. It is perfectly legitimate for us to
question API and naming decisions.

I don't think it's fair to call my responses "snarky".  On the other hand, your suggestion that I view Argument Clinic as "my toy project" is obviously meant as an insult.

It is fair to call my responses "dismissive", because I find nothing compelling about your arguments.

I would color your final sentence slightly.  Yes, there are plenty of legitimate discussions about API semantics and nomenclature.  But there are infinitely many pointless and time-wasting discussions.  Therefore not every question about API and naming decisions is de facto legitimate.

I don't mean to say that your bringing it up originally was unreasonable, that was entirely appropriate.  But surely I've made it clear by now: I find no strength to your arguments, and I'm not changing the name based on them.  At this point all you're accomplishing is making me angry.

The name "nullable" has been in continuous in Argument Clinic since very early on.  It has always been in the PEP, and was in the original prototype published Dec 3 2012.  Nobody has said anything about it, until you, just now, on this issue.  I suggest the reason nobody has said anything before is because the name is fine.


A clarification over something you said previously: you suggested that the name would "confuse Python developers".  Just to be crystal clear: the name is not exposed to Python *users*.  Right now it's solely visible to CPython core developers.  If at some point Argument Clinic is  deemed a public tool, it would also be visible to people writing third-party extension modules.  I am not proposing we use the term "nullable" in user-facing documentation or APIs.  (I happen to think it would be fine for users too, but there's utterly no point in having that debate now.)  For what it's worth, I like to think CPython core developers are not so easily confused as you suggest.

Also: Python draws inspiration from lots of places.  The syntax for decorators was borrowed from Java autodoc comments (and Java annotations, though I think it actually slightly predates those).  I'm not aware of any existing Python precedent we can draw on, so drawing inspiration from C# and SQL strikes me as perfectly appropriate.  On the other hand, I see no precedent bolstering your suggestion.

You yourself told someone in IRC, when they were pestering me about some Argument Clinic bikeshedding, "let's leave Larry alone".  I appreciated it at the time.  I'd appreciate it if you'd follow that advice now too.

> I would color your final sentence slightly.  Yes, there are plenty of
> legitimate discussions about API semantics and nomenclature.  But
> there are infinitely many pointless and time-wasting discussions.
> Therefore not every question about API and naming decisions is de
> facto legitimate.

This is a fair position, thanks.

> At this point all you're accomplishing is making me angry.

Argument Clinic discussions and activity seem to annoy me more than they
bring me anything, so I'll avoid involving myself in it any further.

Here's a fresh patch.  After discussing with Martin at EuroPython, I moved the implementation into Python/getargs.c, and the prototypes into modsupport.h.  I'm still using the example of "repeat.new" to show what it looks like and how it works.  However I don't plan on checking the changes to repeat.new in when I check in the patch--I'd want to discuss it with Raymond first.

Whoops, here's the patch.

FWIW, I agree with Antoine that Nullable Int is a misnomer.

Also, I don't buy into this mission to change signatures for optional ints into int-or-none.   In particular, itertools.repeat() is fine as-is.  Optional int APIs have been around for a long time, they work fine, and they really don't need to be changed.  AFAICT, there is no real issue here that warrants changes to existing APIs.


>>> s = [10, 20, 30]
>>> s.pop(None)
Traceback (most recent call last):
  File "<pyshell#5>", line 1, in <module>
    s.pop(None)
TypeError: 'NoneType' object cannot be interpreted as an integer


>>> int('42', None)
Traceback (most recent call last):
  File "<pyshell#7>", line 1, in <module>
    int('42', None)
TypeError: 'NoneType' object cannot be interpreted as an integer

FWIW, Martin agreed with me at EuroPython that "nullable int" is the proper name.  So does Wikipedia:

    http://en.wikipedia.org/wiki/Nullable_type


As for "repeat_new": like I said, I don't intend to check in this change when checking in nullable ints.  (I really just used it because I was doing the work on a plane and just didn't feel like finding a different example.)  I'm simply not interested in picking a fight about it.

However, this is using the general approach endorsed by Guido:

    https://mail.python.org/pipermail/python-dev/2014-January/131673.html

And indeed, see thread for further specific discussion of repeat_new.


I feel the position of "these have been in Python for years, they're fine" misses the point.  Builtins never had signatures before.  Now that they can have signatures, the design decision of writing builtins with non-Pythonic signatures has come home to roost.  How can the signature for itertools.repeat() convey that "times" is an optional parameter without it having a default value?

I view giving all builtins in Python valid signatures as a worthwhile goal unto itself.  So ISTM that allowing "times" to accept None to mean "repeat forever" is reasonable: it makes the function more Pythonic, and it means it can have a meaningful signature.  But if this goal doesn't interest you, I can see why you'd think this was a waste of time.

Larry, ISTM that you are bulldozing your way through something that isn't an actual problem to be solved.

>  I can see why you'd think this was a waste of time.

I don't think it is just a waste of time;  I think it is a bad idea.  You have a very strong notion of how function signatures should look (i.e. the number of arguments being irrelevant) and you want to impose your ideas on existing, stable APIs for zero benefit.   

> I view giving all builtins in Python valid signatures
> as a worthwhile goal unto itself.

I can already model the behavior of repeat() using *args and **kwds, just like I can for int(), list.pop(), range(), and slice().

You don't seem to get that those tools already work, that people understand them, that they've been stable for a long time, and that they don't need to change for any reason other than that you've worked yourself into a snit about it.

If this feature is contentious, then a PEP is need to decide on it. As it turns out, PEP 436 already discusses the nullable feature. But, as it also turns out, PEP 436 hasn't been accepted yet (at least I could not find any records on that having happened, and the PEP says that it is in draft state).

So what I think needs to happen next is:
- opposition to the PEP or certain features should be recorded in the PEP
- a BDFL delegate needs to be nominated, or Guido needs to declare that he wants to rule on the PEP himself
- a decision on the PEP needs to be made.