Skip to content

DOC: Don't show traceback for :okexcept: #48394

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

Closed
wants to merge 1 commit into from

Conversation

StefRe
Copy link
Contributor

@StefRe StefRe commented Sep 5, 2022

Draft to see if docs build on CI

  • use local modified ipython_directive to add optional no_traceback
    argument to :ok_except: which prevents the (sometimes long) traceback
    from being printed (default is no argument, i.e. the traceback is
    printed) and enable the @okexcept pseudodecorator so that it can be
    applied to individual instructions instead of the whole block
  • use local modified ipython_console_highlighting to enable syntax
    highlighting of exceptions without a traceback
  • fix pylint warnings for ipython_directive.py and simplify
    "".join(["."] * x) to "." * x

The # noqa: E999 after @okexcept no_traceback is necessary to
prevent flake8-rst from complaining about syntax errors as it
interprets the pseudodecorator as a real python decorator.

In ipython_directive.py:1106, pylint complains about access to
self.content before its definition although it is defined in the base
class
. That's why I added # pylint: disable=access-member-before-definition
to make the pre-commit hook pass.

Items 1 and 2 can be reverted if ipython/ipython#13751
gets merged upstream.

Example of the new docs including correct syntax highlighting with the no_traceback option:
image


@StefRe StefRe force-pushed the DOC/no_traceback branch 2 times, most recently from 7c88620 to 6695460 Compare September 5, 2022 15:18
@mroeschke mroeschke added Docs Error Reporting Incorrect or improved errors from pandas labels Sep 6, 2022
@StefRe StefRe force-pushed the DOC/no_traceback branch 2 times, most recently from d70d6a6 to 4431375 Compare September 7, 2022 15:58
@StefRe
Copy link
Contributor Author

StefRe commented Sep 7, 2022

Seems to work, the only thing I don't understand is why we don't get the color output for the exception class, see the created docs artifact:
image

I made a minimal example and the error message gets nicely formatted according to the chosen theme (I also tested the example with sphinx 4.5.0 (used on CI) instead of 5.1.1 and with the pydata_sphinx_theme:
image).

Any thoughts on this @mroeschke? Would it be OK to have the error message without color formatting?

@mroeschke
Copy link
Member

Would it be OK to have the error message without color formatting?

Yeah I think it would be fine if there was no color to start. Could be addressed later

@StefRe
Copy link
Contributor Author

StefRe commented Sep 8, 2022

I found the culprit: IPython.sphinxext.ipython_console_highlighting uses IPyLexer to highlight the code and the traceback. A traceback is recognized with the following regex:

ipytb_start = re.compile(r'^(\^C)?(-+\n)|^(  File)(.*)(, line )(\d+\n)')

So this will need to be changed to something like this to work for exceptions without preceding traceback:

import builtins

exceptions = [name for name, value in builtins.__dict__.items() if isinstance(value, type) and issubclass(value, Exception) and not issubclass(value, Warning)]
ipytb_start = re.compile(r'^(\^C)?(-+\n)|^(  File)(.*)(, line )(\d+\n)|^(' + '|'.join(exceptions) + '): \S.*\n')

grafik

@mroeschke
Copy link
Member

So this will need to be changed to something like this to work for exceptions without preceding traceback:

Do you think this is something that can fixed upstream in iPython?

@StefRe
Copy link
Contributor Author

StefRe commented Sep 8, 2022

Do you think this is something that can fixed upstream in iPython?

Let's see if ipython/ipython#13751 gets merged.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Thanks @StefRe, really nice. lgtm, let's see if we can get the changes merged upstream

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@StefRe
Copy link
Contributor Author

StefRe commented Oct 12, 2022

Please update and respond to this comment if you're still interested in working on this.

Yes, still interested in it. Rebased onto current master. Waiting for ipython/ipython#13751 to be reviewed.

@mroeschke mroeschke removed the Stale label Oct 12, 2022
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Nov 12, 2022
@StefRe
Copy link
Contributor Author

StefRe commented Nov 26, 2022

Please update and respond to this comment if you're still interested in working on this.

Yes, still interested in it. Rebased onto current master. Waiting for ipython/ipython#13751 to be reviewed.
IPython appears to igonore any PR from outside their organization: since this PR of Sep 8, 31 other PR were merged, practically all of them from IPython's organization with the exception of minor fixes of typos in the docs etc. So I'm not sure if it makes sense to keep waiting for review of this PR.

@StefRe StefRe force-pushed the DOC/no_traceback branch 2 times, most recently from 08315f6 to f7447e8 Compare November 27, 2022 10:10
- use local modified ipython_directive to add optional no_traceback
  argument to :ok_except: which prevents the (sometimes long) traceback
  from being printed (default is no argument, i.e. the traceback is
  printed) and enable the @okexcept pseudodecorator so that it can be
  applied to individual instructions instead of the whole block
- use local modified ipython_console_highlighting to enable syntax
  highlighting of exceptions without a traceback
- fix pylint warnings for ipython_directive.py and simplify
  "".join(["."] * x) to "." * x

The "# noqa: E999" after "@okexcept no_traceback" is necessary to
prevent flake8-rst from complaining about syntax errors as it
interprets the pseudodecorator as a real python decorator.

In ipython_directive.py:1106, pylint complains about access to
self.content before its definition although it is defined in the base
class. That's why I added "# pylint: disable=access-member-before-definition"
to make the pre-commit hook pass.

Items 1 and 2 can be reverted if github.com/ipython/ipython/pull/13751
gets merged upstream.
@StefRe
Copy link
Contributor Author

StefRe commented Nov 27, 2022

As the PR ipython/ipython#13751 to update the ipython_directive and ipython_console_highlighting in IPython doesn't seem to be merged or even reviewed anytime soon, I suggest using vendored versions of these directives. Is this OK with you?

If so, my plan is to:

  1. Remove the changes to categorical.rst from this PR so that this PR covers only the directives and can be easily reverted if the PR in IPython should be merged.
    Should the text about these directives removed in Remove references to previously-vendored Sphinx extensions #46624 be included back into https://github.com/pandas-dev/pandas/tree/main/doc/sphinxext?
  2. Update DOC: Clarify sorting and order of categoricals and fix typo #47846 using the no_traceback option.
  3. Make a new PR to replace try/except blocks and static code blocks with :ok_except: and add no_traceback to existing :ok_except: cases with long tracebacks.

@mroeschke
Copy link
Member

Since this PR is pending on ipython/ipython#13751, going to close this for now and we can reopen when that PR is accepted

@mroeschke mroeschke closed this Jul 7, 2023
@mroeschke mroeschke added the Mothballed Temporarily-closed PR the author plans to return to label Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Error Reporting Incorrect or improved errors from pandas Mothballed Temporarily-closed PR the author plans to return to
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: limit error traceback to one line for expected exceptions
4 participants