ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress #4870

ghost · 2014-01-25T14:38:21Z

2nd try on #4866, upstreaming ipython_directive enhancements from pandas.

Report any error or warning encountered during ipython:: block execution to stdout.
Tells you (since yesrterday) the location of the block that triggered the error.
New :okexcept: and :okwarning: options to allow the user to supress such output on per-block basis.

Exceptions raised in ipython blocks are currently not reported and silently end up in
the rendered output. Similarly for Warnings.

Example output:

reading sources... [  0%] basics                                                                        

>>>-------------------------------------------------------------------------
Warning in /home/user1/src/pandas/doc/source/basics.rst at block ending on line 706
Specify :okwarning: as an option in the ipython:: block to suppress this message
----------------------------------------------------------------------------
/home/user1/src/pandas/pandas/core/frame.py:2798: FutureWarning: TimeSeries broadcasting along DataFrame index by default is deprecated. Please use DataFrame.<op> to explicitly broadcast arithmetic operations along the index
  FutureWarning)
<<<-------------------------------------------------------------------------

…warning: options to suppress

Carreau · 2014-01-25T15:10:57Z

IPython/sphinxext/ipython_directive.py

@@ -245,13 +246,15 @@ def block_parser(part, rgxin, rgxout, fmtin, fmtout):
 class EmbeddedSphinxShell(object):
    """An embedded IPython instance to run inside Sphinx"""

-    def __init__(self, exec_lines=None):
+    def __init__(self, exec_lines=None,state=None):


space after coma. :-)

PEP8 is the bane of my existence.

Pushed. There's a lot of PEP8 stuff here, you want the full storm? I consider it just diff noise.

Carreau · 2014-01-25T15:18:49Z

IPython/sphinxext/ipython_directive.py

@@ -421,6 +428,34 @@ def process_input(self, data, input_prompt, lineno):
        elif is_semicolon: # get spacing right
            ret.append('')

+        # context information
+        filename = self.state.document.current_source
+        lineno = self.state.document.current_line


self.state could be None I think, this would then not work.

Mixing maybe ... not sure, will re-read. Was confused by kipped lines on github.

Carreau · 2014-01-25T15:26:17Z

Usually we fix pep-8 only on code we touch.
(Also on github you can put ?w=1 to ignore whitespace in diff, but irrelevant)

Carreau · 2014-01-25T15:30:40Z

So , self.state is used in other places, but my lack of knowledge in the area make me unsure wether or not in the place I pointed it could be None. I would have to dig deeper in it, but test passes, and if you are bringing that from panda, it should work there too.

ghost · 2014-01-25T15:41:23Z

self.state is a sphinx/docutil thing and although It's passed through a new kw with default value None,
in the context of ipython_directive.py It's always passed to the EmbeddedSphinxShell.
I've had to propagate it since it's the bearer of context on the error (filename/lineno).

Haven't had much experience with sphinx extensions, I admit. I just sort of fell into this
from recent issues at pandas.

Carreau · 2014-01-25T15:47:55Z

Fair, enough.
I'm just considering failling early if state is not given or is None.

ghost · 2014-01-25T15:50:06Z

I'm game, what do you suggest?

Carreau · 2014-01-25T15:59:10Z

I don't know, what does @takluyver think ? he did review the other PR, so might have better input than me on this one.

ghost · 2014-01-25T16:13:07Z

I added a commit to handle that just in case, I'm open to any and all other suggestions.

takluyver · 2014-01-25T19:59:16Z

This looks OK to me, but I have no special knowledge of Sphinx extensions. I think pandas is the biggest user of ipython_directive, so if you say changes are working for you, I'm fairly happy to merge them.

ghost · 2014-01-25T20:06:35Z

We've been using :okexcept: for a long time, and it's been fine. It didn't report
where the problem code actually is, which induced some grey hairs among the maintainers.
I added that yesterday and bursts of hysterical euphoria were heard in the corridors
of pandas central.

I think it's solid. If you want to wait a little while we dogfood, no problem.

ghost · 2014-01-25T20:09:06Z

I wonder how well this works for other kernels such as IRuby.
It makes IPython a killer application for writing library documentation.

takluyver · 2014-01-25T20:17:31Z

OK, let's give it a couple of days for pandas to spot any major flaws. If you don't let us know of any issues, we'll merge it then.

At present, I think this uses a shell object within the process, so it wouldn't work with another kernel. It could be rewritten to use the kernel architecture.

chebee7i · 2014-01-27T05:43:45Z

It's not strictly necessary to pass in state, as it can be accessed from self.directive.state. This is actually how I accessed the source file and line numbers in process_output. So access is a bit inconsistent in the code and should probably be made similar. Here's a snippet from the section I'm referring to:

        if self.directive is None:
            source = 'Unavailable'
            content = 'Unavailable'
        else:
            source = self.directive.state.document.current_source
            content = self.directive.content
            # Add tabs and join into a single string.
            content = '\n'.join([TAB + line for line in content])

chebee7i · 2014-01-27T05:51:26Z

Also, it looks like line numbers are not report when raising an exception on a failed doctest. Note to self or anyone who wants to do it: This should probably be added in.

ghost · 2014-01-27T06:01:48Z

I had to scan the file several times until I understood where self.directive is coming from... You hid it well.

ghost · 2014-01-27T06:04:10Z

We don't use doctests in pandas, she's all yours.

chebee7i · 2014-01-27T06:06:22Z

Eck. Not proud of that one...probably should have passed it to __init__ explicitly.

ghost · 2014-01-27T06:07:26Z

fwiw, I felt bad about passing in the state arg. Sometimes you can't win.

ghost · 2014-01-27T06:19:18Z

Added some comments re https://github.com/ipython/ipython/pull/4803/files#r9180526

chebee7i · 2014-01-27T19:33:16Z

(I'm no longer sure where this comment should be. In this PR or in the merged #4803)

Got it, but I suppose that means the fix is valid only if the value of ipython_rgxout has something like a colon and a \s+ immediately after it. While it is certainly the most common case, its probably not a general solution right? Does the Cython magic/parser depend on this exact regex as well?

ghost · 2014-01-27T19:43:09Z

There might be some confusion here, mine or yours I'm not sure.

ipython_rgxout is hardcoded in the file, so the phrase "if the value of ipython_rgxout.." strikes
me as odd. Are you a follower of Hume? If not, Is arbitrary ipython_rgxout a real use case?

The problem is this:

ip_d strips ipython prompts from code
for the first, non-continuation, line it strips the space after the colon.
for continuation lines it does not.
This doesn't seem to break python code (it may be compensated for elsewhere)
It does break cython code.

That's my understanding of what I fixed, being 3rd on the scene and having no insight
into what was previous done, and just wanting to get the hell on with life.

Certainly, I may have misunderstood the problem and fixed it by mistake. Convince me.

also, we might actually be talking about ipython_rgxin, same thing.

ghost · 2014-01-27T19:47:41Z

You may want to look at the original fix in pandas, which I scrapped in favor of fixing
the problem at what I though was it's origin. If there's another level to go, please point it out.

chebee7i · 2014-01-27T23:52:17Z

So ipython_rgxout (and ipython_rgxin for that matter) is configurable in the latest IPython. See: https://github.com/y-p/ipython/blob/PR_ipython_directive/IPython/sphinxext/ipython_directive.py#L858 and also the docstring at the top of the module. But, the continuation prompt is hardcoded at L199 and L384, so perhaps this whole discussion is moot.

To the question of whether or not its a real use case, my guess is that most projects will not use it. But if changing the prompt were so uncommon, then why does IPython allow it in the first place? Support for customized prompts was put in only to be consistent with this feature of IPython.

Practically, maybe the thing to do is just leave your fix as is and wait for people with customized prompts to complain. My guess is that the fix would cause problems only for customized prompts that do not have a space after the colon (if there is even a colon). Would a comment in the code making this clear be sufficient? That is, something stating that the spacing test is designed to work with the default output prompt and might cause problems with customized prompts.

ghost · 2014-01-28T00:00:04Z

Done.

takluyver · 2014-01-28T00:45:45Z

My take is that customising your prompt in the shell makes sense, but documentation should be written with the default prompt.

ghost · 2014-01-30T05:04:10Z

I've compiled the docs many times over the last few days and unless there are further
code review issues I think this can safely be merged.

Heads up, there's another PR coming with unicode/encoding fixes which should be
the last one on this.

chebee7i · 2014-01-30T17:03:41Z

Seems ready.

Aside: should IPython move the content of http://matplotlib.org/sampledoc/ipython_directive.html to its own documentation? We can expand it and keep it updated then too.

takluyver · 2014-01-30T17:39:10Z

Thanks guys, merging.

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress

ipython_directive, report except/warn in block and add :okexcept: :ok…

8e7a5d1

…warning: options to suppress

Carreau reviewed Jan 25, 2014
View reviewed changes

PEP8 fix

58e488e

Carreau reviewed Jan 25, 2014
View reviewed changes

Guard against self.state is None

052280d

ghost assigned takluyver Jan 25, 2014

Remove explicit passing state arg in favor of self.directive.state

a847421

Document previous fix re cython magic support in ipython_directive

6448bf2

add additional clarification in ip_d re prompt stripping

445ecde

Document :okexcept:, :okwarning: in file header

95a512a

takluyver added a commit that referenced this pull request Jan 30, 2014

Merge pull request #4870 from y-p/PR_ipython_directive

bbb0792

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress

takluyver merged commit bbb0792 into ipython:master Jan 30, 2014

jorisvandenbossche mentioned this pull request Feb 20, 2014

DOC: use Sphinx extension directly from numpy/ipython instead of maintaining our own version pandas-dev/pandas#5221

Closed

takluyver mentioned this pull request May 8, 2014

IPython sphinx extension not showing verbatim output #5776

Closed

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request ipython#4870 from y-p/PR_ipython_directive

120950a

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress #4870

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress #4870

ghost commented Jan 25, 2014

Carreau Jan 25, 2014

ghost Jan 25, 2014

ghost Jan 25, 2014

Carreau Jan 25, 2014

Carreau Jan 25, 2014

Carreau commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

takluyver commented Jan 25, 2014

ghost commented Jan 25, 2014

ghost commented Jan 25, 2014

takluyver commented Jan 25, 2014

chebee7i commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 28, 2014

takluyver commented Jan 28, 2014

ghost commented Jan 30, 2014

chebee7i commented Jan 30, 2014

takluyver commented Jan 30, 2014

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress #4870

ipython_directive, report except/warn in block and add :okexcept: :okwarning: options to suppress #4870

Conversation

ghost commented Jan 25, 2014

Carreau Jan 25, 2014

Choose a reason for hiding this comment

ghost Jan 25, 2014

Choose a reason for hiding this comment

ghost Jan 25, 2014

Choose a reason for hiding this comment

Carreau Jan 25, 2014

Choose a reason for hiding this comment

Carreau Jan 25, 2014

Choose a reason for hiding this comment

Carreau commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

Carreau commented Jan 25, 2014

ghost commented Jan 25, 2014

takluyver commented Jan 25, 2014

ghost commented Jan 25, 2014

ghost commented Jan 25, 2014

takluyver commented Jan 25, 2014

chebee7i commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 27, 2014

ghost commented Jan 27, 2014

chebee7i commented Jan 27, 2014

ghost commented Jan 28, 2014

takluyver commented Jan 28, 2014

ghost commented Jan 30, 2014

chebee7i commented Jan 30, 2014

takluyver commented Jan 30, 2014