ENH: propagating attrs always uses deepcopy

[timhoffm]

Always using a deepcopy prevents shared state and thus unintentional modification of the attrs of other objects. IMHO this safety has a higher priority than the slight performance cost of the deepcopy. The implementation now skips the copying if attrs are not used (i.e. an empty dict). This check takes only ~20ns. Thus, the attrs copy mechanism has no performance impact if attrs are not used.

• Closes BUG: copy.deepcopy() doesn't deepcopy the metadata in .attrs #54134
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

[timhoffm]

Importing stdlib copy at the top of generic.py issues the a number of pylint warnings of the type:

pandas/core/generic.py:289:8: W0621: Redefining name 'copy' from outer scope (line 5) (redefined-outer-name)

What is the preferred way in the project to handle this?

[mroeschke]

What is the preferred way in the project to handle this?

Probably using from copy import deepcopy should be fine

[mroeschke]

Just a pre-commit check failing otherwise looks good

[mroeschke]

Thanks @timhoffm

[rhshadrach]

This patch may have induced a performance regression. If it was a necessary behavior change, this may have been expected and everything is okay.

Please check the links below. If any ASVs are parameterized, the combinations of parameters that a regression has been detected for appear as subbullets.

• finalize.Finalize.time_finalize_micro
  • series=<class 'pandas.core.series.Series'>
  • series=<class 'pandas.core.frame.DataFrame'>

Subsequent benchmarks may have skipped some commits. The link below lists the commits that are between the two benchmark runs where the regression was identified.

6a83910...364c9cb

[timhoffm]

I believe the detected changes come from this PR.

The test uses obj.attrs, which this PR changes to deepcopy in finalize. finalize performance should not decrease if attrs are not used (in fact, it might be marginally faster), but I don't see whether you have an ASV test for that as well.

Note also that the tests use attrs with 1000 entries, which makes the deepcopy cost quite prominent. I expect that typical attrs is a more like a bit of metadata with << 1000 entries.

The use of deepcopy is expected to be a bit slower. I'd argue that it's a necessary safety measure to prevent multiple objects to share the same attrs value objects. It might otherwise be quite surprising that a user changes attrs down the road of operations, and by that change the attrs of an input object. Such a coupling is even more surprising nowadays that dataframe values themselves can be considered independent because of copy-on-write.
IMHO, if the user does not want to afford the cost of the safety net, they should not use attrs but mange that state themselves outside of pandas. - Or if there is really interest make shallow copy available as an opt-in.