Accordion style merge issue fix

[ElijahAhianyo]

return the var is the var to be merged is None

[masenf]

the hesitation here is that the function now has two different behaviors:

1. if other is not None, then we get a new Var
2. if other is None, then we get the same Var

Code that is using ._merge shouldn't have to worry about whether the returned value can be safely mutated or not.

To keep both code paths consistent, i'd suggest return self._replace(), so that _merge always returns a new Var

[ElijahAhianyo]

the hesitation here is that the function now has two different behaviors:

1. if other is not None, then we get a new Var
2. if other is None, then we get the same Var

Code that is using ._merge shouldn't have to worry about whether the returned value can be safely mutated or not.

To keep both code paths consistent, i'd suggest return self._replace(), so that _merge always returns a new Var

Got it, thats interesting, what are the repercussions in returning same var vs new var though?

[masenf]

what are the repercussions in returning same var vs new var though

Var is a mutable object; so if i call ._merge and then change _var_is_local or some other attribute, i wouldn't expect that to actually change any of the input Var.

This is fresh in my mind because I just fixed #2421 which was a result of almost this same thing, except in the .create path, where if the object you're creating a Var from is already a Var, then the original object is returned directly as-is. I didn't change how .create works in that PR, because it's been like that for a long time, but we should avoid introducing new ways to shoot one's foot in a similar vein.