why not

kwargs['unsafe'] = unsafe

?

Hi @gst,
This was done to emulate how passing unsafe via kwargs was originally designed.
That is, if unsafe is not explicitly set, kwargs don't default unsafe to False, it's just not present in kwargs.
There are also several test cases already written where, if unsafe is not set then kwargs is expected not to contain the unsafe key.
Particularly in this bug fix, I didn't want to introduce any behavioral changes.

well I think both of us got their points actually :)

def __init__(
            self, getter, attribute, new, spec, create,
            spec_set, autospec, new_callable, kwargs, *, unsafe=False
        ):

given there is none annotation to the kwargs we can, eventually, do anything with it.
might be it seems to be a regular dict/mapping though.

and using regular dict[index] = value looks the way to do it ?

I agree, the problem as you are pointing out is in the initializer where unsafe is pulled out and made into a named argument.
My original thought was to just change the initializer and remove that unsafe from it. That is probably the best solution and how it was originally.

But then I realized all the public/exposed APIs like for example:

def patch(
        target, new=DEFAULT, spec=None, create=False,
        spec_set=None, autospec=None, new_callable=None, *, unsafe=False, **kwargs
):
def _patch_object(
        target, attribute, new=DEFAULT, spec=None,
        create=False, spec_set=None, autospec=None,
        new_callable=None, *, unsafe=False, **kwargs
    ):

have unsafe also made into an independent named argument. Since it is these public/exposed APIs that get called **kwargs at this point is always a dict and that get passed internally everywhere in the mock module.

It is strange why kwargs is not checking for None in the _patch initializer(so one can test _patch class independently), and I see that some initializer calls go into great extend to ensure that an empty dict is passed into the initializer. Like the following piece of code that can be found in the _patch_multiple function.

this_patcher = _patch(
    getter, attribute, new, spec, create, spec_set,
    autospec, new_callable, {}
)

So as it is right now, the initializer for _patch class is some what coupled with the calling public API in the sense that whoever calls it has to always pass a dict for kwargs. It's a little unsafe but could just have been the design from the beginning.

So if we always get a dict for kwargs then why not use,
kwargs['unsafe'] = unsafe instead of kwargs.update({'unsafe': unsafe})?
You are right, it's probably more efficient to use dict[index] = value style here.
However the if conditional statement that checks if unsafe is set to True will need to stay. Otherwise we will change the behavior of kwargs and that break some testcases written in other modules.
Would that be okay?

Instead of asserting that the internal attribute is set I guess it's good to test the actual behavior. The other tests above check for AttributeError. Maybe this can just call the patched object and try using an incorrect assert helper like assret_called_once.

from unittest.mock import patch

class Something:

    def meth(self):
        pass


with patch(f"{__name__}.Something.meth", unsafe=True, autospect=True) as patched_object:
    patched_object()
    patched_object.assret_called_once()

• In master branch

./python bpo94497.py
Traceback (most recent call last):
  File "/home/karthikeyan/stuff/python/cpython/bpo94497.py", line 11, in <module>
    patched_object.assret_called_once()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/karthikeyan/stuff/python/cpython/Lib/unittest/mock.py", line 643, in __getattr__
    raise AttributeError(
AttributeError: 'assret_called_once' is not a valid assertion. Use a spec for the mock if 'assret_called_once' is meant to be an attribute.. Did you mean: 'assert_called_once'?

• With patch no AttributeError is raised since like the behavior before Python 3.10

Thanks @tirkarthi.
You are absolutely right.
I was so focused on the bug, I forgot the testcase was to test the behavior for spelling mistakes, which is rightfully what I should have been testing from the beginning.
I uploaded a new commit fix it.

I feel like this commit doesn't require an entry in Misc/NEWS.d/next/, but I'm not sure how to set a "skip news" label on the commit.