Mutable function parameter defaults?

[altendky]

https://github.com/python-desert/desert/blob/e201af413c2453e8cf5418443e72623ed650427a/src/desert/__init__.py#L11-L13

https://github.com/python-desert/desert/blob/e201af413c2453e8cf5418443e72623ed650427a/src/desert/__init__.py#L27-L29

I haven't looked into this yet but it made me think of https://github.com/python-attrs/attrs/issues/278.

[python-desert]

I don't think we want to mutate our argument. We could add some tests to make sure that we don't mutate it. If we're confident that we're not mutating it then I'm happy for the default to be a dict.

[altendky]

How is that better than the normal incantation for this? And why isn't some static check noticing this?

[altendky]

Regardless the decision, there's already an =None so something ought to get changed for consistency.

https://github.com/python-desert/desert/blob/fc89fa35c3d29409a53a303de6ddaa1c4b52627b/src/desert/_make.py#L171-L173

[python-desert]

Saving some links for future readers:

• https://cosmicpercolator.com/2016/01/16/mutable-default-arguments-are-your-friend/
• https://bugs.python.org/issue38654

[altendky]

Keep in mind that the problem in https://github.com/python-attrs/attrs/issues/278 had nothing to do with attrs mutating anything. I called attr.ib() and then mutated the metadata attribute's dict. Not only are all existing results of attr.ib() affected but all future ones as well. That shows the issue can come up related to a high quality code base (referring to attrs, not mine :]) and also that the issue can manifest in unexpected ways external to the immediate code around the default and completely outside the library's code. Bonus, this is even in code highly related to usage of desert. :]

The BP issue just says 'there is no bug' which is true, but besides the point.

The blog says 'just be careful to not mutate' which is true, but 'just be careful' about hard things is hardly an argument against simple practices where getting them wrong is identified by existing tooling and getting them right avoids a whole class of risk.

Now instead of the normal =None approach there is also the arg = dict(arg) option that provides isolation of both the default and any passed thing from any unintended mutation. Though it does cost in situations where people want to be able to post-mutate themselves. But, this is just a passing thought at the moment that may not warrant significant allowance for.

import attr

@attr.s
class C:
    _d = attr.ib()

d = {}
c = C(d=d)
d['k'] = 37
assert c._d == d

(edited the assertion to refer to c._d not c.d)

I suspect there are more options and considerations to think through and maybe tooling could be written to help (verify the mutable default (or any parameter) is not mutated and never gets stored anywhere either) but the 'just do not mutate and mutable defaults are ok' argument does not seem particularly strong to me since it does not address the not-so-simple risks.

[python-desert]

Keep in mind that the problem in python-attrs/attrs#278 had nothing to do with attrs mutating anything. I called attr.ib() and then mutated the metadata attribute's dict.

Good point.

mapping = dict(mapping) seems good. I think we could test that two fields' metdata dicts is not each other.

I'm not understanding your code snippet though.

[altendky]

Note my correction in the snippet, though I think it's not really terribly relevant.

Re: the code snippet, there might be some expectation that if I pass a dict (d) in to a thing (C) that puts 'that information' onto a result (c._d) then it's fairly common that you can mutate d after the fact and have it affect c._d. I'm not sure this is a good expectation, but I haven't thought about it enough to decide it is trivial or not worth consideration. mapping = dict(mapping) (yes, same as I suggested) does not accommodate the snippet working.