Skip to content

traceback: py313 updates #12032

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

Merged
merged 4 commits into from
May 27, 2024
Merged

traceback: py313 updates #12032

merged 4 commits into from
May 27, 2024

Conversation

AlexWaygood
Copy link
Member

A bunch of functions also have new **kwargs parameters in Python 3.13. I deliberately haven't added these, as the only additional keyword argument you should be passing is _colorize -- and that's a private implementation detail that deliberately isn't exposed.

This comment has been minimized.

Comment on lines 89 to 100
@overload
def format_exception_only(exc: BaseExceptionGroup[BaseException], /, *, show_group: bool = False) -> list[str]: ...
@overload
def format_exception_only(
exc: Unused, /, value: BaseExceptionGroup[BaseException], *, show_group: bool = False
) -> list[str]: ...
@overload
def format_exception_only(exc: BaseException | None, /, *, show_group: Literal[False] = False) -> list[str]: ...
@overload
def format_exception_only(
exc: Unused, /, value: BaseException | None, *, show_group: Literal[False] = False
) -> list[str]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need - or should - special case show_group. If show_group is True, but the exception is not an exception group, it will be printed as normal. If it's False, but it is an exception group, only a single exception will be printed - and that is fine. I think it was a deliberate design decision that this works either way, but cc @iritkatriel.

Also, format_exception_only now has a colorize option, defaulting to False, which has weirdly being hidden in the kwargs.

Copy link
Member Author

@AlexWaygood AlexWaygood May 27, 2024

Choose a reason for hiding this comment

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

I agree we don't need to special-case show_group. I added some special-casing because it seemed to me like it was a case similar to others in typeshed, where we alert users to code that wouldn't necessarily generate an exception at runtime, but would be useless. AFAIK, there's no possible reason to pass show_group=True if the object being passed in is not known to be an ExceptionGroup instance, so it seems indicative of a probable bug if a user does that -- therefore something that a type checker could usefully complain about.

Having said all that, however, I also wasn't sure that this was worth the added complexity here, so I'll remove the special-casing for show_group.

Also, format_exception_only now has a colorize option, defaulting to False, which has weirdly being hidden in the kwargs.

Yes, but I discussed in the PR description why I was deliberately not adding the private _colorize or colorize keyword arguments to any of the traceback functions in this PR. See e.g. python/cpython#111567 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK, there's no possible reason to pass show_group=True if the object being passed in is not known to be an ExceptionGroup instance, so it seems indicative of a probable bug if a user does that -- therefore something that a type checker could usefully complain about.

I have to admit that my understanding is limited, but I think the point is that if you have a BaseException you don't know whether it's a group or not. And maybe you want to format all group members in case it's a group:

try:
    ...  # might raise an exception group or single exceptions
except BaseException as exc:
    format_exception_group(exc, show_group=True)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, that's a fair point

@AlexWaygood AlexWaygood requested a review from srittau May 27, 2024 11:41
@AlexWaygood AlexWaygood merged commit de0fc0a into python:main May 27, 2024
62 of 63 checks passed
@AlexWaygood AlexWaygood deleted the 313-traceback branch May 27, 2024 11:56
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

max-muoto pushed a commit to max-muoto/typeshed that referenced this pull request Sep 8, 2024
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.

2 participants