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

PEP 678: del err.__notes__, and why we're not just using a list #2360

Merged
merged 3 commits into from
Feb 25, 2022

Conversation

Zac-HD
Copy link
Contributor

@Zac-HD Zac-HD commented Feb 24, 2022

Final (🤞) update to the PEP, before I ask the steering council to consider it 🙂

- a new method ``BaseException.add_note(note, *, replace=False)``,
- ``BaseException.__notes__``, a read-only field which is a tuple of zero or
- a new method ``BaseException.add_note(note)``,
- ``BaseException.__notes__``, a read-only attribute which is a tuple of zero or
Copy link
Member

Choose a reason for hiding this comment

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

Is it still correct to say it’s read-only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, since it's not writable or mutable - del clears it, but that's a pretty complicated and rare caveat to express in a sentence here.

Copy link
Member

Choose a reason for hiding this comment

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

del clearing it surely makes it mutable? You could say "an attribute exposing a read-only tuple", as conceptually one could argue it is a new tuple every time it is changed.

Equally I think this is quite minor and your intent is clear from the text.

A

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

One minor punctuation fix, otherwise LGTM

pep-0678.rst Outdated Show resolved Hide resolved
Co-authored-by: CAM Gerlach <[email protected]>
@Zac-HD
Copy link
Contributor Author

Zac-HD commented Feb 25, 2022

I think this is read for merge 🙂

@JelleZijlstra
Copy link
Member

So you don't want to change the "read-only attribute" part? I never know what the eyes emoji means.

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Feb 25, 2022

Oops, yeah. That was "I need to think about this", and having thought I think it's OK as-is. Happy to change it if someone has a suggestion though.

@JelleZijlstra JelleZijlstra merged commit 7d3a7b9 into python:main Feb 25, 2022
@JelleZijlstra
Copy link
Member

Sounds good, if you change your mind we can change it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants