NotificationsYou must be signed in to change notification settings
Fork3.8k
Star8.4k

Delete rect subscripts should error not segfault#3440

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

illume merged 1 commit intomainfromankith26-rect-del-fixes

Sep 12, 2022

Merged

Delete rect subscripts should error not segfault#3440

illume merged 1 commit intomainfromankith26-rect-del-fixes

Sep 12, 2022

Conversation

Copy link

Contributor

ankith26 commentedSep 10, 2022

Simple PR tofix#3438

Delete rect subscripts should error not segfault

26d3e1c

Starbuck5 approved these changes

Sep 10, 2022

View reviewed changes

Copy link

Contributor

Starbuck5 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I see that this is almost exactly how Vectors do it, so this code isn't some crazy new thing. 😄

Copy link

Contributor

Starbuck5 commentedSep 10, 2022•
edited
Loading

I see some other places useAttributeError instead of type error, what do you think? Like the other deletion code in Rect, and some deletion code in Color. Maybe it makes sense the way it is, since right now it seems to be consistent AttributeError if referring to the attribute by name, TypeError if attempting to delete as part of a sequence.