PEP 678: Clarify how the notes tuple is updated and when it is copied

[Zac-HD]

Following python/steering-council#105 (comment).

[CAM-Gerlach]

One comment on clarity.

[Zac-HD]

OK, I've pushed a (I sincerely hope it's) final half-sentence specifying that add_note(note) behaves like __notes__ += (note,) (allocates a new tuple on change, not access, never mutates in place). Should be ready to merge.

[CAM-Gerlach]

Mean to do so earlier, but LGTM from a PEP editor perspective, pending approval by @iritkatriel .

[iritkatriel]

LGTM

[gvanrossum]

I'm moving some discussion here since I don't think it belongs in python/steering-council#105 (I don't like having a substantive discussion in the SC tracker).

@iritkatriel wrote:

This was the actual change in the pep (https://github.com/python/peps/pull/2377/files).

(Note that that is this PR.)

Do you think it’s over specifying how notes is updated?

I really want this to grow the size of exception objects by only one pointer.

The current formulation (as of this PR) then pretty much forces us into this implementation:

class BaseException:
    ...
    __notes: tuple[str, ...] | None = None  # Private variable

    def add_note(self, note: str):
        if self.__notes is None:
            self.__notes = (note,)
        else:
            self.__notes = (*self.__notes__, note)

    @property
    def __notes__(self) -> tuple[str, ...] | None:
        return self.__notes

    @__notes__.deleter
    def __notes__(self):
        self.__notes = None

I'd wish to allow add_note() to use an internal data structure that's a list or a linked list or whatever is deemed most efficient given usage patterns, and have __notes__ construct a fresh tuple on the fly without having to cache it (since the cache would extend the size of the object by another pointer, i.e. 8 bytes). Never mind that's unlikely what the initial implementation will be, changing the implementation shouldn't violate the PEP. But currently the PEP text seems to imply that consecutive calls to the property (without intervening add_notes() call) are required to return the same tuple object (which my implementation sketch does, but which I think the PEP needn't require).

[JelleZijlstra]

@Zac-HD wrote above:

Hmm, that's a little surprising because it makes the obvious test for ExceptionGroup.derive() (a.__notes__ is b.__notes__) fail.

But I'm not sure what this test is or why it's obvious.

[Zac-HD]

If you split an ExceptionGroup, it should have the same notes; so people will check that and some will use is instead of ==. I'd left that unspecified until I started getting asked about it - maybe we should instead say explicitly that it's not specified?

[gvanrossum]

People shouldn’t compare tuple by identity any more than they should integers.

[iritkatriel]

In that case we can remove the last commit from python/cpython#31317 and then we have the notes in a list and we create a new tuple each time it’s accessed.

The pep doesn’t need to say all of this, but to Brett’s point it needs to say that the tuple you got from .__notes__ is not mutated by a subsequent .add_note(). Is this correct?

[JelleZijlstra]

but to Brett’s point it needs to say that the tuple you got from .__notes__ is not mutated by a subsequent .add_note().

That feels unnecessary. Tuples can't be mutated (unless you do something really crazy in C), so there's no need to spell that out.

[gvanrossum]

That feels unnecessary. Tuples can't be mutated (unless you do something really crazy in C), so there's no need to spell that out.

That was my starting point too, but Brett pushed back specifically because "in C you can do anything." So it seems we need to somehow make that clear without forcing an implementation.

I personally think it's fine to put notes in a list and tuplify on demand without caching. (But I also think it's fine to put them in a tuple, creating a fresh tuple in add_note().)