Skip to content

Add 'make eval'. Fix crash in pprint(). Reformat some files.#237

Merged
bmerkle merged 10 commits intomicrosoft:mainfrom
gvanrossum:evals
Apr 13, 2026
Merged

Add 'make eval'. Fix crash in pprint(). Reformat some files.#237
bmerkle merged 10 commits intomicrosoft:mainfrom
gvanrossum:evals

Conversation

@gvanrossum
Copy link
Copy Markdown
Collaborator

Apparently at least one of isort and black was updated and now produces more compact code in some cases.

Also change the black call in Makefile to use only -tpy312 since black on 3.12 cannot parse back the py314 code it generated (maybe only the last -t flag was effective?).

And finally make Makefile more robust by using uv run X instead of .venv/bin/X.

Copy link
Copy Markdown
Contributor

@KRRT7 KRRT7 left a comment

Choose a reason for hiding this comment

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

Thanks for catching the format_code crash — that's on us from #235. Two notes:

  1. format_code fallback wraps text in quotespprint.pformat(text) on a string returns its repr(), so a non-literal like <CustomClass object at 0x...> becomes '<CustomClass object at 0x...>' with surrounding quotes in the output. Since text is already a repr string meant for display, returning it directly would be cleaner (inline comment below).

  2. pretty_print silently drops prefix/suffix — This is a separate regression from #235 that isn't addressed here. Our PR changed the body to print(pprint.pformat(obj, width=line_width)), dropping the prefix/suffix args. Callers like utils.pretty_print(actual1, Fore.GREEN, Fore.RESET) lost their color output. Should be print(prefix + pprint.pformat(obj, width=line_width) + suffix). Happy to send a follow-up PR for this if you'd like.

return pprint.pformat(text, width=line_width)


def reindent(text: str) -> str:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pprint.pformat(text) on a plain string returns its repr() — e.g. '<CustomClass object at 0x...>' with surrounding quotes. Since text is already a repr string intended for terminal/diff output, returning it directly avoids the extra quoting:

Suggested change
def reindent(text: str) -> str:
except (ValueError, SyntaxError):
# Fall back to the raw text if it's not a valid literal
return text

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That suggestion would make the code incorrect.

@gvanrossum
Copy link
Copy Markdown
Collaborator Author

Ah, thanks. If you add the fixes to this PR using the GitHub suggestion feature I will apply them.

@KRRT7
Copy link
Copy Markdown
Contributor

KRRT7 commented Apr 13, 2026

The format_code suggestion is up as an inline comment. The pretty_print fix can't be a GitHub suggestion since it's not in this PR's diff — here's the change for copy-paste if you'd like to include it:

In src/typeagent/aitools/utils.py, pretty_print currently ignores prefix/suffix:

def pretty_print(obj: object, prefix: str = "", suffix: str = "") -> None:
    """Pretty-print an object using pprint."""
    import pprint

    line_width = min(200, shutil.get_terminal_size().columns)
    print(pprint.pformat(obj, width=line_width))

Should be:

def pretty_print(obj: object, prefix: str = "", suffix: str = "") -> None:
    """Pretty-print an object using pprint."""
    import pprint

    line_width = min(200, shutil.get_terminal_size().columns)
    print(prefix + pprint.pformat(obj, width=line_width) + suffix)

Otherwise happy to send a separate PR for it.

return pprint.pformat(ast.literal_eval(text), width=line_width)
except (ValueError, SyntaxError):
# Fall back to simple pprint of the string itself if it's not a valid literal
return pprint.pformat(text, width=line_width)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return pprint.pformat(text, width=line_width)
return text

pprint.pformat on a plain string wraps it in quotes — returning text directly avoids that.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Let's put a pin in that until we see its effect. (In any case you would have to update the comment too.)

Regression from microsoft#235: pretty_print stopped using the prefix/suffix
parameters, so callers passing color codes (e.g. Fore.GREEN) lost
their colored output.
@KRRT7
Copy link
Copy Markdown
Contributor

KRRT7 commented Apr 13, 2026

Opened gvanrossum#1 against your evals branch with the pretty_print prefix/suffix fix — one-click merge if you want it.

@bmerkle bmerkle self-assigned this Apr 13, 2026
Copy link
Copy Markdown
Collaborator Author

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

@KRRT7 Let's just merge this as-is now -- it fixes the prefix/suffix and the rest is debatable.

return pprint.pformat(ast.literal_eval(text), width=line_width)
except (ValueError, SyntaxError):
# Fall back to simple pprint of the string itself if it's not a valid literal
return pprint.pformat(text, width=line_width)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Let's put a pin in that until we see its effect. (In any case you would have to update the comment too.)

return pprint.pformat(text, width=line_width)


def reindent(text: str) -> str:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That suggestion would make the code incorrect.

Copy link
Copy Markdown
Collaborator

@bmerkle bmerkle left a comment

Choose a reason for hiding this comment

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

LGTM

@bmerkle bmerkle merged commit 434a224 into microsoft:main Apr 13, 2026
16 checks passed
@gvanrossum gvanrossum deleted the evals branch April 13, 2026 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants