-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[bugfix] solve crash when using inspect()
on the "pyparsing" package
#2294
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
from typing import Any, Iterable, Optional, Tuple | ||
|
||
from .console import Group, RenderableType | ||
from .control import escape_control_codes | ||
from .highlighter import ReprHighlighter | ||
from .jupyter import JupyterMixin | ||
from .panel import Panel | ||
|
@@ -19,12 +20,6 @@ def _first_paragraph(doc: str) -> str: | |
return paragraph | ||
|
||
|
||
def _reformat_doc(doc: str) -> str: | ||
"""Reformat docstring.""" | ||
doc = cleandoc(doc).strip() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that we were ending up calling |
||
return doc | ||
|
||
|
||
class Inspect(JupyterMixin): | ||
"""A renderable to inspect any Python Object. | ||
|
||
|
@@ -161,11 +156,9 @@ def safe_getattr(attr_name: str) -> Tuple[Any, Any]: | |
yield "" | ||
|
||
if self.docs: | ||
_doc = getdoc(obj) | ||
_doc = self._get_formatted_doc(obj) | ||
if _doc is not None: | ||
if not self.help: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The process of calling |
||
_doc = _first_paragraph(_doc) | ||
doc_text = Text(_reformat_doc(_doc), style="inspect.help") | ||
doc_text = Text(_doc, style="inspect.help") | ||
doc_text = highlighter(doc_text) | ||
yield doc_text | ||
yield "" | ||
|
@@ -200,13 +193,10 @@ def safe_getattr(attr_name: str) -> Tuple[Any, Any]: | |
add_row(key_text, Pretty(value, highlighter=highlighter)) | ||
else: | ||
if self.docs: | ||
docs = getdoc(value) | ||
docs = self._get_formatted_doc(value) | ||
if docs is not None: | ||
_doc = _reformat_doc(str(docs)) | ||
if not self.help: | ||
_doc = _first_paragraph(_doc) | ||
_signature_text.append("\n" if "\n" in _doc else " ") | ||
doc = highlighter(_doc) | ||
_signature_text.append("\n" if "\n" in docs else " ") | ||
doc = highlighter(docs) | ||
doc.stylize("inspect.doc") | ||
_signature_text.append(doc) | ||
|
||
|
@@ -220,3 +210,24 @@ def safe_getattr(attr_name: str) -> Tuple[Any, Any]: | |
f"[b cyan]{not_shown_count}[/][i] attribute(s) not shown.[/i] " | ||
f"Run [b][magenta]inspect[/]([not b]inspect[/])[/b] for options." | ||
) | ||
|
||
def _get_formatted_doc(self, object_: Any) -> Optional[str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you mind adding a docstring. |
||
""" | ||
Extract the docstring of an object, process it and returns it. | ||
The processing consists in cleaning up the doctring's indentation, | ||
taking only its 1st paragraph if `self.help` is not True, | ||
and escape its control codes. | ||
|
||
Args: | ||
object_ (Any): the object to get the docstring from. | ||
|
||
Returns: | ||
Optional[str]: the processed docstring, or None if no docstring was found. | ||
""" | ||
docs = getdoc(object_) | ||
if docs is None: | ||
return None | ||
docs = cleandoc(docs).strip() | ||
if not self.help: | ||
docs = _first_paragraph(docs) | ||
return escape_control_codes(docs) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ | |
from functools import partial, reduce | ||
from math import gcd | ||
from operator import itemgetter | ||
from rich.emoji import EmojiVariant | ||
from typing import ( | ||
TYPE_CHECKING, | ||
Any, | ||
|
@@ -141,15 +140,16 @@ def __init__( | |
tab_size: Optional[int] = 8, | ||
spans: Optional[List[Span]] = None, | ||
) -> None: | ||
self._text = [strip_control_codes(text)] | ||
sanitized_text = strip_control_codes(text) | ||
self._text = [sanitized_text] | ||
self.style = style | ||
self.justify: Optional["JustifyMethod"] = justify | ||
self.overflow: Optional["OverflowMethod"] = overflow | ||
self.no_wrap = no_wrap | ||
self.end = end | ||
self.tab_size = tab_size | ||
self._spans: List[Span] = spans or [] | ||
self._length: int = len(text) | ||
self._length: int = len(sanitized_text) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's the bugfix itself! 🙂 i.e. we should use the length of the sanitised text, rather than the one of the text we received as an argument |
||
|
||
def __len__(self) -> int: | ||
return self._length | ||
|
@@ -394,9 +394,10 @@ def plain(self) -> str: | |
def plain(self, new_text: str) -> None: | ||
"""Set the text to a new value.""" | ||
if new_text != self.plain: | ||
self._text[:] = [new_text] | ||
sanitized_text = strip_control_codes(new_text) | ||
self._text[:] = [sanitized_text] | ||
old_length = self._length | ||
self._length = len(new_text) | ||
self._length = len(sanitized_text) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same bug here 🙂 |
||
if old_length > self._length: | ||
self._trim_spans() | ||
|
||
|
@@ -906,10 +907,10 @@ def append( | |
|
||
if len(text): | ||
if isinstance(text, str): | ||
text = strip_control_codes(text) | ||
self._text.append(text) | ||
sanitized_text = strip_control_codes(text) | ||
self._text.append(sanitized_text) | ||
offset = len(self) | ||
text_length = len(text) | ||
text_length = len(sanitized_text) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...and here |
||
if style is not None: | ||
self._spans.append(Span(offset, offset + text_length, style)) | ||
self._length += text_length | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -299,3 +299,41 @@ class Thing: | |
"╰──────────────────────────────────────────╯\n" | ||
) | ||
assert render(module, methods=True) == expected | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"special_character,expected_replacement", | ||
( | ||
("\a", "\\a"), | ||
("\b", "\\b"), | ||
("\f", "\\f"), | ||
("\r", "\\r"), | ||
("\v", "\\v"), | ||
), | ||
) | ||
def test_can_handle_special_characters_in_docstrings( | ||
special_character: str, expected_replacement: str | ||
): | ||
class Something: | ||
class Thing: | ||
pass | ||
|
||
Something.Thing.__doc__ = f""" | ||
Multiline docstring | ||
with {special_character} should be handled | ||
""" | ||
|
||
expected = """\ | ||
╭─ <class 'tests.test_inspect.test_can_handle_sp─╮ | ||
│ class test_can_handle_special_characters_in_do │ | ||
│ cstrings.<locals>.Something(): │ | ||
│ │ | ||
│ Thing = class Thing(): │ | ||
│ Multiline docstring │ | ||
│ with %s should be handled │ | ||
╰────────────────────────────────────────────────╯ | ||
""" % ( | ||
expected_replacement | ||
) | ||
|
||
assert render(Something, methods=True) == expected | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to make sure that we don't have regressions on this bugfix, before trying to solve it I started by writing a test that reproduce the issue : this test was crashing before the fix, and passes now 🎈 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while working on this I noticed that creating a Python script with a name starting ending with
test_
anywhere in the project's repo was collected by Pytest: I think we'd better tell Pytest to stick with the contents of ourtests/
folder - which should also make the tests collection faster as a side effect