Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore some functionality of the sphinx directive. #11402

Merged
merged 5 commits into from Oct 18, 2018

Conversation

@Carreau
Copy link
Member

@Carreau Carreau commented Oct 16, 2018

See #11362

The issue is in 2 part, before IPython 7.0 the input splitter was
state full, this was (in part) due to readline.

The second part is that because of this, we had to be a bit adressive of
what was considered complete code (it had to have 2 new line). This is
now not required anymore as we can submit stuff as a whole.

I hope that this fixes that. I have another fix in mind that count (and
reset) the number of consecutive blank line, but that will be more
complicated end code.

See ipython#11362

The issue is in 2 part, before IPython 7.0 the input splitter was
state full, this was (in part) due to readline.

The second part is that because of this, we had to be a bit adressive of
what was considered complete code (it had to have 2 new line). This is
now not required anymore as we can submit stuff as a whole.

I hope that this fixes that. I have another fix in mind that count (and
reset) the number of consecutive blank line, but that will be more
complicated end code.
@TomAugspurger
Copy link

@TomAugspurger TomAugspurger commented Oct 16, 2018

This fixed things for the small test in #11362

I'm giving it a try on pandas docs.

Loading

@TomAugspurger
Copy link

@TomAugspurger TomAugspurger commented Oct 16, 2018

It seems like exceptions fail the build. With the same setup from #11362,

# index.rst
Welcome to test's documentation!
================================

.. ipython:: python

   def f(x):
       x = 2
       raise ValueError('bad')
       return x

   f(2)

.. ipython:: python

   print('hi')


Testing

then make html results in

Running Sphinx v1.8.1
making output directory...
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 1 source files that are out of date
updating environment: 1 added, 0 changed, 0 removed
reading sources... [100%] index

>>>-------------------------------------------------------------------------
Exception in /Users/taugspurger/sandbox/sphinx-test/index.rst at block ending on line 12
Specify :okexcept: as an option in the ipython:: block to suppress this message
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-2-c510dc86724b> in <module>
----> 1 f(2)

<ipython-input-1-fe8b2c77be70> in f(x)
      1 def f(x):
      2     x = 2
----> 3     raise ValueError('bad')
      4     return x

ValueError: bad
<<<-------------------------------------------------------------------------


Exception occurred:
  File "/Users/taugspurger/sandbox/IPython/IPython/sphinxext/ipython_directive.py", line 562, in process_input
    raise RuntimeError('Non Expected exception in `{}` line {}'.format(filename, lineno))
RuntimeError: Non Expected exception in `/Users/taugspurger/sandbox/sphinx-test/index.rst` line 12
The full traceback has been saved in /var/folders/hz/f43khqfn7b1b1g8z_z6y3bsw0000gp/T/sphinx-err-6psc98eb.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [html] Error 2

I I include okexcept as an argument to the directive, things do finish. IIRC, the behavior on 6.x was to print a warning to stdout with a message telling you to use okexcept.

Loading

@Carreau
Copy link
Member Author

@Carreau Carreau commented Oct 16, 2018

I I include okexcept as an argument to the directive, things do finish. IIRC, the behavior on 6.x was to print a warning to stdout with a message telling you to use okexcept.

Yes but that was changed explicitley in #11321 (should maybe have been better documented) to actually start to catch regression in documentation....

I can try to make it configurable though. One other discussion was to make okexcept fail if there is actually no exception...

Loading

@Carreau
Copy link
Member Author

@Carreau Carreau commented Oct 16, 2018

You can now set ipython_strict_fail = False in your config.py to get back previous behavior.

if you have a better name...

Loading

@TomAugspurger
Copy link

@TomAugspurger TomAugspurger commented Oct 16, 2018

Thanks, that name seems fine, but if you want to match Sphinx I think it would be ipython_warning_is_error to match warning-is-error.

Loading

The behavior pre-6.5 was to keep on going even if unexpected exceptions
or warnings were shown, on 7.x the default is to abort the build.

For compat reasons (and convenience), you can now set back the behavior
to the original one to just log to stderr and move on.
@Carreau
Copy link
Member Author

@Carreau Carreau commented Oct 16, 2018

ipython_warning_is_error to match warning-is-error

Done,

SyntaxError is now detected as a error as well, I restored the old documentation from 0.13... that may need some polish.

Loading

Copy link

@choldgraf choldgraf left a comment

In general it looks quite good - I have a few thoughts for clarifying/structuring the documentation!

Loading


.. note::

This has been salvadged from history, so information may be approximate or

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just say something like "The IPython Sphinx Directive is in 'beta' and currently under active development. Improvements to the code or documentation are welcome!"

Loading

doctest the output. The inputs are fed to an embedded ipython
session and the outputs from the ipython session are inserted into
your doc. If the output in your doc and in the ipython session don't
match on a doctest assertion, an error will be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error will be...

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Carreau @choldgraf can I replace "error will be... " for "an error will occur"?

Loading

Copy link
Member Author

@Carreau Carreau Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, feel free to make any replacement you feel is better.

Loading

In [1]: x = 'hello world'

# this will raise an error if the ipython output is different
@doctest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is @doctest explicitly documented somewhere? or only in reference here?

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could someone please tell me more about @doctest? Where is it documented?

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I see there's a definition at the bottom of the file. But some more info on it would be nice, I find it a little confusing.

Loading

SyntaxError: invalid syntax


The embedded interpreter supports some limited markup. For example,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd split each of these sub-sections by a header. E.g.

Persisting the Python session across IPython directive blocks
=============================================================

...

Adding documentation tests to your IPython directive
====================================================

...

Multi-line input
================

etc

Loading


You can do doctesting on multi-line output as well. Just be careful
when using non-deterministic inputs like random numbers in the ipython
directive, because your inputs are ruin through a live interpreter, so

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*run

Loading


.. ipython:: python
@savefig plot_simple_python.png width=4in

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this plot a figure (because of @savefig) or will it plot nothing (because of the semicolon)?

Loading

ioff()
ion()
Multi-line input is handled.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People could confuse this with multi-line input in the ipython section above. Maybe call this "splitting Python statements across lines"?

Loading

works'
print(line.split('&'))
Functions definitions are correctly parsed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have a section called "syntax highlighting" and just step through several major use-cases there. E.g.:

Syntax highlighting
===================

The Sphinx IPython directive can highlight most Python structures and patterns. This section shows some of the most common ones.

Multi-line statements
---------------------
EXAMPLE CODE

Function definitions
--------------------
EXAMPLE CODE

etc

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @choldgraf , I don't know if I understood quite well what you meant here.

Do you mean creating another section with a whole set example code blocks or rename an existing section for Syntax Highlighting and use the code that has already been written?

Thank you

Loading

Copy link
Member Author

@Carreau Carreau Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think chris was suggesting refactoring and being clearer with heading/subheading.

You can see the rendered version there, There is a lot of duplicate information and organisational issues.

Loading

Also, can be applied to the entire ``.. ipython`` block as a
directive option with ``:verbatim:``.
@savefig OUTFILE [IMAGE_OPTIONS]
Copy link

@choldgraf choldgraf Oct 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok - so the decorators are here. In that case you should always link to this section whenever you mention a decorator above. Something like for more information on decorator usage, see :ref:`directive_decorators` and then add the label `.. _directive_decorators:` to this section

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to create some kind of a link? I didn't understand much how am I supposed to add this reference. But good idea, this certainly helps.

Loading

Copy link
Member Author

@Carreau Carreau Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that is just an example on how to use savefig. Do what you can, even partially. If it's annoying or confusing send what you have and feel free to move on to something more interesting.

Loading

Copy link
Contributor

@LucianaMarques LucianaMarques Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Carreau, I think I had this doubt because I was not familiar with reStructureText until today. Thank you for your reply!

Loading

Configuration Options
=====================

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify that these are the configurations you'd provide underneath the directive with :option:

Loading

@Carreau Carreau added this to the 7.1 milestone Oct 18, 2018
@Carreau
Copy link
Member Author

@Carreau Carreau commented Oct 18, 2018

Thanks @choldgraf for the feedback, I've open #11410 as a easy first issue to follow up on.

I'm going to get that in as it fixes some functionalities.

Loading

@Carreau Carreau merged commit 1695193 into ipython:master Oct 18, 2018
4 checks passed
Loading
@Carreau
Copy link
Member Author

@Carreau Carreau commented Oct 21, 2018

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants