[Python-Dev] PEP 553

Mon Oct 2 17:36:59 EDT 2017

On Mon, Oct 2, 2017 at 11:02 AM, Barry Warsaw <barry at python.org> wrote:

> Thanks for the review Guido!  The PEP and implementation have been updated
> to address your comments, but let me briefly respond here.
>
> > On Oct 2, 2017, at 00:44, Guido van Rossum <guido at python.org> wrote:
>
> > - There's a comma instead of a period at the end of the 4th bullet in
> the Rationale: "Breaking the idiom up into two lines further complicates
> the use of the debugger,” .
>
> Thanks, fixed.
>
> > Also I don't understand how this complicates use
>
> I’ve addressed that with some additional wording in the PEP.  Basically,
> it’s my contention that splitting it up on two lines introduces more
> opportunity for mistakes.
>
> > TBH the biggest argument (to me) is that I simply don't know *how* I
> would enter some IDE's debugger programmatically. I think it should also be
> pointed out that even if an IDE has a way to specify conditional
> breakpoints, the UI may be such that it's easier to just add the check to
> the code -- and then the breakpoint() option is much more attractive than
> having to look up how it's done in your particular IDE (especially since
> this is not all that common).
>
> This is a really excellent point!  I’ve reworked that section of the PEP
> to make this clear.
>
> > - There's no rationale for the *args, **kwds part of the breakpoint()
> signature. (I vaguely recall someone on the mailing list asking for it but
> it seemed far-fetched at best.)
>
> I’ve added some rationale.  The idea comes from optional `header` argument
> to IPython’s programmatic debugger API.  I liked that enough to add it to
> pdb.set_trace() for 3.7.  IPython accepts other optional arguments, so I
> think we do want to allow those to be passed through the call chain.  I
> expect any debugger’s advertised entry point to make these optional, so
> `breakpoint()` will always just work.
>
> > - The explanation of the relationship between sys.breakpoint() and
> sys.__breakpointhook__ was unclear to me
>
> I think you understand it correctly, and I’ve hopefully clarified that in
> the PEP now, so you wouldn’t have to read the __displayhook__ (or
> __excepthook__) docs to understand how it works.
>
> > - Some pseudo-code would be nice.
>
> Great idea; added that to the PEP (pretty close to what you have, but with
> the warnings handling, etc.)
>
> > I think something like `os.environ['PYTHONBREAKPOINT'] = 'foo.bar.baz';
> breakpoint()` should result in foo.bar.baz() being imported and called,
> right?
>
> Correct.  Clarified in the PEP now.
>
> > - I'm not quite sure what sort of fast-tracking for PYTHONBREAKPOINT=0
> you had in mind beyond putting it first in the code above.
>
> That’s pretty close to it.  Clarified.
>
> > - Did you get confirmation from other debuggers? E.g. does it work for
> IDLE, Wing IDE, PyCharm, and VS 2015?
>
> From some of them, yes.  Terry confirmed for IDLE, and I posted a
> statement in favor of the PEP from the PyCharm folks.  I’m pretty sure
> Steve confirmed that this would be useful for VS, and I haven’t heard from
> the Wing folks.
>
> > - I'm not sure what the point would be of making a call to breakpoint()
> a special opcode (it seems a lot of work for something of this nature).
> ISTM that if some IDE modifies bytecode it can do whatever it well please
> without a PEP.
>
> I’m strongly against including anything related to a new bytecode to PEP
> 553; they’re just IMHO orthogonal issues, and I’ve removed this as an open
> issue for 553.
>
> I understand why some debugger developers want it though.  There was a
> talk at Pycon 2017 about what PyCharm does.  They have to rewrite the
> bytecode to insert a call to a “trampoline function” which in many ways is
> the equivalent of breakpoint() and sys.breakpointhook().  I.e. it’s a small
> function that sets up and calls a more complicated function to do the
> actual debugging.  IIRC, they open up space for 4 bytecodes, with all the
> fixups that implies.  The idea was that there could be a single bytecode
> that essentially calls builtin breakpoint().  Steve indicated that this
> might also be useful for VS.
>
> There’s a fair bit that would have to be fleshed out to make this idea
> real, but as I say, I think it shouldn’t have anything to do with PEP 553,
> except that it could probably build on the APIs we’re adding here.
>
> > - I don't see the point of calling `pdb.pm()` at breakpoint time. But
> it could be done using the PEP with `import pdb; sys.breakpointhook =
> pdb.pm` right? So this hardly deserves an open issue.
>
> Correct, and I’ve removed this open issue.
>
> > - I haven't read the actual implementation in the PR. A PEP should not
> depend on the actual proposed implementation for disambiguation of its
> specification (hence my proposal to add pseudo-code to the PEP).
> >
> > That's what I have!
>
> Cool, that’s very helpful, thanks!
>

I've seen your updates and it is now acceptable, except for *one* nit: in
builtins.breakpoint() the pseudo code raises RuntimeError if
sys.breakpointhook is missing or None. OTOH sys.breakpointhook() just
issues a RuntimeWarning when something's wrong with the hook. Maybe
builtins.breakpoint() should also just warn if it can't find the hook?
Setting `sys.breakpointhook = None` might be the simplest way to
programmatically disable breakpoints. Why not allow it?

To Terry: Barry has given another excellent argument for passing through
*args, **kwds so I remove my objection to it, regardless of what I think of
your argument about the IDLE debugger and root windows. (It's been too long
since I used Tkinter, so I don't trust my intuition there much anyways.)

-- 
--Guido van Rossum (python.org/~guido)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20171002/a85404db/attachment-0001.html>