Le 06/03/2019 à 13:53, Chris Angelico a écrit :
On Wed, Mar 6, 2019 at 11:18 PM Brice Parent email@example.com wrote:
The major implication to such a modification of the Dict.update method, is that when you're using it with keyword arguments (by opposition to passing another dict/iterable as positional), you're making a small non-backward compatible change in that if in some code, someone was already using the keyword that would be chosing (here "on_collision"), their code would be broken by the new feature. Anyway, if the keyword is slected wisely, the collision case will almost never happen, and be quite easy to correct if it ever happened.
You can make it unlikely, yes, but I'd dispute "easy to correct". Let's suppose that someone had indeed used the chosen keyword (and remember, the more descriptive the argument name, the more likely that it'll be useful elsewhere and therefore have a collision). How would they discover this? If they're really lucky, there MIGHT be an exception (if on_collision accepts only a handful of keywords, and the collision isn't one of them), but if your new feature is sufficiently flexible, that might not happen. There'll just be incorrect behaviour.
As APIs go, using specific keyword args at the same time as **kw is a bit odd. Consider:
button_options.update(button_info, on_click=frobnicate, style="KDE", on_collision="replace")
It's definitely not obvious which of those will end up in the dictionary and which won't. Big -1 from me on that change.
That's indeed a good point. Even if the correction is quite easy to make in most cases. With keyword only changes:
button_options.update(dict(on_click=frobnicate, style="KDE", on_collision="replace")) # or button_options.update(dict(on_collision="replace"), on_click=frobnicate, style="KDE")
In the exact case you proposed, it could become a 2-liners:
button_options.update(button_info) button_options.update(dict(on_click=frobnicate, style="KDE", on_collision="replace"))
In my code, I would probably make it into 2 lines, to make clear that we have 2 levels of data merging, one that is general (the first), and one that is specific to this use-case (as it's hard written in the code), but not everyone doesn't care about the number of lines.
But for the other part of your message, I 100% agree with you. The main problem with such a change is not (to me) that it can break some edge cases, but that it would potentially break them silently. And that, I agree, is worth a big -1 I guess.