Skip to content
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

make CanonicalIndexError an Exception type #47008

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

cjdoris
Copy link
Contributor

@cjdoris cjdoris commented Oct 2, 2022

It appears CanonicalIndexError was mistakenly not declared to be a subtype of Exception, so I have fixed this and added a test.

Should be backported to 1.8, where CanonicalIndexError was added.

Aside: it may be worth adding some generic type-hierarchy tests. e.g. any *Error type should be an Exception. Or @test_throws FooError ... could check that FooError <: Exception (this is how I came across this in the first place).

Verified

This commit was signed with the committer’s verified signature.
Byron Sebastian Thiel
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Thanks! Good catch.

@LilithHafner LilithHafner added merge me PR is reviewed. Merge when all tests are passing backport 1.8 Change should be backported to release-1.8 labels Oct 2, 2022
@LilithHafner LilithHafner merged commit 7085703 into JuliaLang:master Oct 2, 2022
@cjdoris cjdoris deleted the canonicalindexerror branch October 2, 2022 12:01
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Oct 6, 2022
KristofferC pushed a commit that referenced this pull request Oct 27, 2022
@KristofferC KristofferC mentioned this pull request Oct 27, 2022
37 tasks
KristofferC pushed a commit that referenced this pull request Oct 28, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Nov 8, 2022
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.

None yet

4 participants