Repository navigation
Argument Clinic: add "nullable ints" #64540
Description
Activity
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 bpo-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 bpo-20189, that signature will always be hidden.
- The type you get back is not int but "nullable_int_t", a structure
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.
vajrasky commented
on Jan 22, 2014 vajraskymannequinMannequin>>> 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(bpo-20185).
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.htmlThere 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
17 remaining items
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_typeAs 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.htmlAnd 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.
loewis commented
on Aug 1, 2014 loewismannequinMannequinIf 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.
There has been no further requests for this feature since 2014/2015. I suggest to close this as wont-implement. If someone really wants to drive this further, they can re-open.
See also #112068.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: