Suggestion: Adding a flag that allows for operating automapper without deepcopy

[g-pichler]

Every time the automapper encounters an object, it does not pass it to the __init__ of the target class directly, but performs a copy.deepcopy() operation at mapper.py, L252. While this behavior is not documented (I did not find it mentioned in README.md), after some deliberation, I do consider it a sane default for an automapper.

But there can be occasions, where this behavior might not be desired. I am facing such a situation currently, where I want the autmapper to simply pass the object directly through to __init__. I did not find a way to achieve this.

My proposed solution is to create an optional parameter for the .map() call that can be used to disable deepcopy(). This will preserve the current behavior, which I believe to be a sane default, but allow direct pass-through. Would you be interested in this feature? You can find a proof-of-concept in my fork.

PS: If you are wondering why I need this feature: I am using automapper to handle database objects created with SQLAlchemy. If the same object is referenced more than once, then deepcopy() is performed more than once and SQLAlchemy complains about duplicates with the same primary key.

[anikolaienko]

Hi @g-pichler ,

That's an interesting observation. As you mentioned, it is a feature and when object is passed as is (without deepcopy) it creates a situation when your source and target objects have same instance of a child object. Which makes it dangerous if any of the two decides to modify that child object.

I see 2 solutions. You can overriding __deepcopy__ method: https://stackoverflow.com/questions/1500718/how-to-override-the-copy-deepcopy-operations-for-a-python-object

Or, as you mentioned, we can add an argument to map function.

As I'm thinking about it overriding __deepcopy__ is a poor solution cause you may need eventually to use the original deepcopy logic somewhere else.

It totally makes sense to implement an argument to a map function that will say something like deepcopy=True by default but user can overwrite it.

Could you create another PR please?

[anikolaienko]

Do you want me to add you to the authors of this project?

[g-pichler]

Hey.

That's an interesting observation. As you mentioned, it is a feature and when object is passed as is (without deepcopy) it creates a situation when your source and target objects have same instance of a child object. Which makes it dangerous if any of the two decides to modify that child object.

Yes, absolutely. I wasn't expecting this behavior, but it makes total sense as a feature. In 99% of the cases, people will either expect/want it or it won't make a difference. I just happened to be in the 1% where it causes problems.

You can overriding __deepcopy__ method: https://stackoverflow.com/questions/1500718/how-to-override-the-copy-deepcopy-operations-for-a-python-object

As I'm thinking about it overriding __deepcopy__ is a poor solution cause you may need eventually to use the original deepcopy logic somewhere else.

Changing the __deepcopy__ logic hadn't even occurred to me. But, I agree with the problem you point out. It is a behavior change of automapper that is required and this would be a rather ugly workaround in my opinion, that could easily backfire.

It totally makes sense to implement an argument to a map function that will say something like deepcopy=True by default but user can overwrite it.

Could you create another PR please?

I am happy that you are interested. I'll prepare a pull request, but this might take some days. It also has the additional benefit that the documentation of the deepcopy=... option will serve as documentation of the default deepcopy behavior. So others won't have to discover it themselves. ;)

[g-pichler]

Do you want me to add you to the authors of this project?

I don't think this is necessary, unless you don't have the time to review my PR. But so far, you have been very responsive. Thanks for that again, by the way.

[anikolaienko]

My pleasure. Let me know if I can help you. I think you can go ahead and create PR from your Fork and we can iterate over it.

[g-pichler]

I just added the pull request. let me know what you think. Sorry for the delay.

[anikolaienko]

sorry for the delay. I have reviewed the code, made some modifications and merged it. I changed the name of the argument to use_deepcopy to avoid variable shadowing or existing deepcopy method. I also made argument as bool and only left in map function. Let me know if you use this argument often. Having it in add method seems a bit dangerous to me, cause the whole deepcopy concept is a bit out of normal workflow concept. User may specify mapping and then don't realize time after that he is actually using mapping with deepcopy disabled.

I really appreciate your contribution @g-pichler ! Let me know your thoughts.

[anikolaienko]

I will close issue for now. Let me know if you really need use_deepcopy in map.add method. Otherwise, let's have another issue created.

[g-pichler]

Hey. Sorry for the long delay. No, I do not need use_deepcopy in map.add(). I only added it there for consistency. But as you pointed out, it might be confusing for a user. For the sake of simplicity, I agree with you. It should be enough to have that option in .map().

The flag name use_deepcopy is more expressive and clear to a user. Good catch on the variable shadowing. My IDE did not warn me about that. But maybe I just overlooked it. I didn't go through all your changes yet. But I will do so and let you know if anything catches my eye. Thanks very much.

[anikolaienko]

No worries, most of it is your work. You may notice some of the formatting is different with your IDE. I have formatting configs in code-checks folder and it's used by pre-commit tool. I still need to setup CI/CD for this repo rather than building every time from local env :(