Skip to content

Remove special handling due to IPython lacking support for exception groups#2702

Merged
Zac-HD merged 6 commits intopython-trio:masterfrom
jakkdl:ipython_exceptiongroup_support
Sep 5, 2023
Merged

Remove special handling due to IPython lacking support for exception groups#2702
Zac-HD merged 6 commits intopython-trio:masterfrom
jakkdl:ipython_exceptiongroup_support

Conversation

@jakkdl
Copy link
Member

@jakkdl jakkdl commented Jul 13, 2023 •
edited
Loading

Quick draft request I'll look over later in case it needs further fixes, but IPython now has a workaround for displaying exceptiongroups, and the comment said this section could be removed in that case. ipython/ipython#13753

EDIT: I guess it needs a release too, and we'd need to bump the IPython requirement to that as well - so this will have to languish for quite a while.

@jakkdl jakkdl changed the title Remove special handling due to IPython lacking support for exception groups [Waiting for new release] Remove special handling due to IPython lacking support for exception groups Jul 13, 2023
@codecov
Copy link

codecov bot commented Jul 13, 2023 •
edited
Loading

Codecov Report

Merging #2702 (51db601) into master (1c15500) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2702      +/-   ##
==========================================
- Coverage   98.94%   98.94%   -0.01%     
==========================================
  Files         113      113              
  Lines       16919    16883      -36     
  Branches     3050     3036      -14     
==========================================
- Hits        16741    16705      -36     
  Misses        123      123              
  Partials       55       55
Files Changed Coverage Δ
trio/_core/_multierror.py 100.00% <100.00%> (ø)
trio/_core/_tests/test_multierror.py 100.00% <100.00%> (ø)

@A5rocks A5rocks changed the base branch from master to autodeps/bump_from_bf9fe6 July 18, 2023 03:15
@A5rocks A5rocks mentioned this pull request Jul 18, 2023
Merged
@A5rocks A5rocks marked this pull request as ready for review July 18, 2023 03:16
@A5rocks
Copy link
Contributor

A5rocks commented Jul 18, 2023

I retargeted this PR to the PR that bumps dependencies and went ahead and marked this as reviewable: I think you will need to remove a test (or at least update it) but I believe this fixes the CI for that other PR.

@A5rocks A5rocks changed the base branch from autodeps/bump_from_bf9fe6 to master July 18, 2023 03:18
@A5rocks A5rocks marked this pull request as draft July 18, 2023 03:18
@A5rocks
Copy link
Contributor

A5rocks commented Jul 18, 2023 •
edited
Loading

Sorry, I just realize I was mistaken. Your fix hasn't gone out, I just jumped the gun when I saw multierrors were formatting differently!

Additionally, it seems I can't force-push my merge above out:

@A5rocks ➜ /workspaces/trio (ipython_exceptiongroup_support) $ git push --force
remote: Permission to jakkdl/trio.git denied to A5rocks.
fatal: unable to access 'https://github.com/jakkdl/trio/': The requested URL returned error: 403

:(

@jakkdl
Copy link
Member Author

jakkdl commented Jul 18, 2023

Sorry, I just realize I was mistaken. Your fix hasn't gone out, I just jumped the gun when I saw multierrors were formatting differently!

Ah sorry, maybe could've been a bit clearer in my edit :)

Additionally, it seems I can't force-push my merge above out:

@A5rocks ➜ /workspaces/trio (ipython_exceptiongroup_support) $ git push --force
remote: Permission to jakkdl/trio.git denied to A5rocks.
fatal: unable to access 'https://github.com/jakkdl/trio/': The requested URL returned error: 403

:(

Added you as a collaborator to the fork, think that might help? I could maybe start pushing branches to the trio/ repository otherwise, but I suspect force-pushing from other people isn't gonna come up too many more times.

@jakkdl jakkdl changed the title [Waiting for new release] Remove special handling due to IPython lacking support for exception groups [Waiting for new IPython release] Remove special handling due to IPython lacking support for exception groups Jul 18, 2023
@A5rocks
Copy link
Contributor

A5rocks commented Jul 18, 2023 •
edited
Loading

Yeah I don't think that'll happen again: I'll try not to jump to conclusions next time!

EDIT: I don't need to join your repo FWIW, you can force push yourself xd (since you're here, I just wanted to decrease the work I caused!)

@jakkdl
Copy link
Member Author

jakkdl commented Jul 18, 2023 •
edited
Loading

Yeah I don't think that'll happen again: I'll try not to jump to conclusions next time!

EDIT: I don't need to join your repo FWIW, you can force push yourself xd (since you're here, I just wanted to decrease the work I caused!)

Ah, you can just push reversion commit[s] then. git revert is very handy

@Zac-HD Zac-HD mentioned this pull request Sep 2, 2023
Closed
8 tasks
@Zac-HD
Copy link
Member

Zac-HD commented Sep 4, 2023 •
edited
Loading

IPython 8.15 has been released with native ExceptionGroup support, so let's rebase this PR and get it in before Trio 0.23 🚀

@TeamSpen210 TeamSpen210 force-pushed the ipython_exceptiongroup_support branch from 00ba87b to 761ec2a Compare September 5, 2023 02:23
@Zac-HD
Copy link
Member

Zac-HD commented Sep 5, 2023

Looks like we just need to delete trio/_core/_tests/test_multierror.py::test_ipython_exc_handler and test_ipython_custom_exc_handler, and add a changelog, then we'll be ready to merge!

(and maybe also enable skip-covered in .coveragerc so it's easier to find out what failed next time)

@TeamSpen210 TeamSpen210 changed the title [Waiting for new IPython release] Remove special handling due to IPython lacking support for exception groups Remove special handling due to IPython lacking support for exception groups Sep 5, 2023
@jakkdl
Copy link
Member Author

jakkdl commented Sep 5, 2023

@TeamSpen210 mind adding a changelog entry as well?

@jakkdl jakkdl marked this pull request as ready for review September 5, 2023 09:47
@jakkdl jakkdl mentioned this pull request Sep 5, 2023
Merged
Zac-HD
Zac-HD approved these changes Sep 5, 2023
View reviewed changes
Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

+5 / -198 🤩

@Zac-HD Zac-HD merged commit bf204c4 into python-trio:master Sep 5, 2023
@jakkdl jakkdl deleted the ipython_exceptiongroup_support branch September 7, 2023 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TeamSpen210 TeamSpen210 TeamSpen210 approved these changes

@Zac-HD Zac-HD Zac-HD approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jakkdl @A5rocks @Zac-HD @TeamSpen210