"Can't pickle" error message changed?

[jpivarski]

In more than one PR, we're starting to see the following error (likely unrelated to those PRs).

=================================== FAILURES ===================================
__________________ test_serialise_with_nonserialisable_attrs ___________________

array_pickler = <module 'pickle' from '/opt/hostedtoolcache/Python/3.12.5/x64/lib/python3.12/pickle.py'>

    def test_serialise_with_nonserialisable_attrs(array_pickler):
        attrs = {**SOME_ATTRS, "non_transient_key": lambda: None}
        array = ak.Array([1, 2, 3], attrs=attrs)
        with pytest.raises(
            (AttributeError, array_pickler.PicklingError), match=r"Can't pickle"
        ):
>           array_pickler.loads(array_pickler.dumps(array))
E           AttributeError: Can't get local object 'test_serialise_with_nonserialisable_attrs.<locals>.<lambda>'

array      = <Array [1, 2, 3] type='3 * int64'>
array_pickler = <module 'pickle' from '/opt/hostedtoolcache/Python/3.12.5/x64/lib/python3.12/pickle.py'>
attrs      = {'foo': '!SOME',
 'non_transient_key': <function test_serialise_with_nonserialisable_attrs.<locals>.<lambda> at 0x7fac6ed2ba60>}

tests/test_2757_attrs_metadata.py:47: AttributeError

During handling of the above exception, another exception occurred:

array_pickler = <module 'pickle' from '/opt/hostedtoolcache/Python/3.12.5/x64/lib/python3.12/pickle.py'>

    def test_serialise_with_nonserialisable_attrs(array_pickler):
        attrs = {**SOME_ATTRS, "non_transient_key": lambda: None}
        array = ak.Array([1, 2, 3], attrs=attrs)
>       with pytest.raises(
            (AttributeError, array_pickler.PicklingError), match=r"Can't pickle"
        ):
E       AssertionError: Regex pattern did not match.
E        Regex: "Can't pickle"
E        Input: "Can't get local object 'test_serialise_with_nonserialisable_attrs.<locals>.<lambda>'"

array      = <Array [1, 2, 3] type='3 * int64'>
array_pickler = <module 'pickle' from '/opt/hostedtoolcache/Python/3.12.5/x64/lib/python3.12/pickle.py'>
attrs      = {'foo': '!SOME',
 'non_transient_key': <function test_serialise_with_nonserialisable_attrs.<locals>.<lambda> at 0x7fac6ed2ba60>}

tests/test_2757_attrs_metadata.py:44: AssertionError

It seems like the error message has changed.

The test was originally added by @agoose77 in #2757 with error message "Can't pickle local object".

awkward/tests/test_2757_attrs_metadata.py

Lines 41 to 45 in e7cccc9

	def test_serialise_with_nonserialisable_attrs(array_pickler):
	attrs = {**SOME_ATTRS, "non_transient_key": lambda: None}
	array = ak.Array([1, 2, 3], attrs=attrs)
	with pytest.raises(AttributeError, match=r"Can't pickle local object"):
	array_pickler.loads(array_pickler.dumps(array))

Then I changed it in #2927 to "Can't pickle":

awkward/tests/test_2757_attrs_metadata.py

Lines 41 to 47 in c54d083

	def test_serialise_with_nonserialisable_attrs(array_pickler):
	attrs = {**SOME_ATTRS, "non_transient_key": lambda: None}
	array = ak.Array([1, 2, 3], attrs=attrs)
	with pytest.raises(
	(AttributeError, array_pickler.PicklingError), match=r"Can't pickle"
	):
	array_pickler.loads(array_pickler.dumps(array))

The new message, "Can't get local object 'test_serialise_with_nonserialisable_attrs..'", is essentially the same thing, though I don't know why the message is changing.

@agoose77, before I arbitrarily change the expected message again (or only catch the exception types and tell ruff that we don't want to constrain the error message), does anything seem wrong here?

[agoose77]

It looks like CPython changed the error message here. We should probably still check the message, but handle both cases (patched CPython and unpatched).

c.f. python/cpython#122386

[jpivarski]

Thanks for finding this! Since this is the second time it's changing, perhaps we shouldn't check the text of the error message at all (which, like __repr__, is intended for the convenience of humans reading the message, and therefore isn't really part of the API). ruff complains about that, but we can ignore it.

[agoose77]

My feeling is that we want to carefully identify the "this doesn't pickle" case against a general failure of pickling that's not due to the lambda. We could even be more fine grained that we are now. So I'd be in favour of widening the test to handle all known variants of the message.

If you want to take a different tradeoff between dev churn and silent errors, then relax the case!

[jpivarski]

Okay, then, I'll make it match=r"(pickle|local object)". That captures all of the previous messages and it's likely that future messages will involve one of the two strings.