bpo-38293: Allow shallow and deep copying of property objects #16438

GudniNatan · 2019-09-27T19:13:25Z

Copying property objects results in a TypeError. Steps to reproduce:

>>> import copy
>>> obj = property()
>>> copy.copy(obj)

This affects both shallow and deep copying.
My idea for a fix is to add property objects to the list of "atomic" objects in the copy module.
These already include types like functions and type objects.

I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :)

https://bugs.python.org/issue38293

Automerge-Triggered-By: @csabella


        Allow shallow and deep copying of property objects + unit tests

eamanu

Good catch @GudniNatan i am not copy expert but I test it and run successfully.

I don't know if will need necessary a NEWS entry, IMO yes, but I would like hear some
core dev opinion.

brandtbucher

Thanks @GudniNatan, and welcome to CPython. This looks good!

I think this should have a NEWS entry. It's a new feature (and there's precedent for it when copy/deepcopy support was added for other objects in the past). Creating one is super quick!

Maybe just something like:

Add :func:`copy.copy` and :func:`copy.deepcopy` support to :func:`property` objects.

Just out of curiosity, what was your use case for wanting to copy these?


        📜🤖 Added by blurb_it.

GudniNatan · 2019-09-29T10:26:04Z

I was experimenting with using property objects to validate data in dataclasses. Here is some code that should help illustrate:

from dataclasses import field, dataclass, asdict, fields


def string_check(x):
    if type(x) != str:
        raise ValueError("Should be a string.")
    return x


def validator_factory(field_name, validator):
    def fget(self):
        return self.__dict__[field_name]

    def fset(self, value):
        self.__dict__[field_name] = validator(value)

    return property(fget, fset)


@dataclass
class MyClass():
    my_value: str = validator_factory("my_value", string_check)


@dataclass
class ClassSchema():
    field_defaults: list


my_class_schema = ClassSchema(
    field_defaults=[f.default for f in fields(MyClass)]
)

print(my_class_schema)
print(asdict(my_class_schema))

The MyClass dataclass has an attribute my_value, and the idea is that if you set it to be anything other than a string, it will throw a ValueError.

We were also using a library (dataclasses-jsonschema) that has some similar functionality to ClassSchema, in that it stores the default values of our dataclasses. It will then try to convert those to dictionaries with the dataclasses asdict function. However, asdict will try to give you a dictionary that is independent of the instance you feed it. To do that it uses the deepcopy, and then you get the TypeError: can't pickle property objects

So basically calling asdict() on certain dataclass instances results in this weird error.

gvanrossum

I have a nagging doubt. You can create a property from any callable, e.g. property(lambda self: None). The argument is not type-checked, so you can also create a property from a non-callable, e.g. property([1, 2, 3]). When we deep-copy such a property, shouldn't we deep-copy the list (i.e. the fget attribute)? After all, when we deep-copy a tuple containing a list, the list gets copied.

@GudniNatan For your use case, is deep-copyability required?

arigo · 2019-10-23T04:41:31Z

@gvanrossum following that argument, copy.copy(func) should not return its original object either, because func is mutable (i.e. a following func.foo = 42 should not appear on its copy). Of course I'm not suggesting that this should be changed; this is only a reminder that copy() and deepcopy() are best-effort hacks in the first place.

gvanrossum · 2019-10-23T05:27:45Z

@arigo That's fair. Tuples are special, because they are meant to be robust data structures, and a tuple containing a list is a well-known (and used) corner case. I'll approve.

miss-islington · 2020-01-12T17:41:52Z

Thanks @GudniNatan for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington · 2020-01-12T17:41:57Z

Sorry @GudniNatan, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 9f3fc6c5b4993f2b362263b494f84793a21aa073 3.8


        bpo-38293: Allow shallow and deep copying of property objects (python…

…GH-16438) Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293 (cherry picked from commit 9f3fc6c) Co-authored-by: Guðni Natan Gunnarsson <1493259+GudniNatan@users.noreply.github.com>

bedevere-bot · 2020-01-12T17:42:13Z

GH-17968 is a backport of this pull request to the 3.7 branch.

miss-islington · 2020-01-12T17:45:28Z

Thanks @GudniNatan for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2020-01-12T17:45:37Z

GH-17969 is a backport of this pull request to the 3.8 branch.


        bpo-38293: Allow shallow and deep copying of property objects (python…

…GH-16438) Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293 (cherry picked from commit 9f3fc6c) Co-authored-by: Guðni Natan Gunnarsson <1493259+GudniNatan@users.noreply.github.com>


        bpo-38293: Allow shallow and deep copying of property objects (GH-16438)

Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293 (cherry picked from commit 9f3fc6c) Co-authored-by: Guðni Natan Gunnarsson <1493259+GudniNatan@users.noreply.github.com>


        Merge pull request #166 from python/master

bpo-38293: Allow shallow and deep copying of property objects (pythonGH-16438)


        bpo-38293: Allow shallow and deep copying of property objects (GH-16438)

Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293 (cherry picked from commit 9f3fc6c) Co-authored-by: Guðni Natan Gunnarsson <1493259+GudniNatan@users.noreply.github.com>

brandtbucher · 2020-01-12T18:13:56Z

Congrats on your first CPython contribution @GudniNatan! 🍾

Looking forward to seeing more from you in the future.


        bpo-38293: Allow shallow and deep copying of property objects (python…

…GH-16438) Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293


        bpo-38293: Allow shallow and deep copying of property objects (python…

…GH-16438) Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293


        bpo-38293: Allow shallow and deep copying of property objects (python…

…GH-16438) Copying property objects results in a TypeError. Steps to reproduce: ``` >>> import copy >>> obj = property() >>> copy.copy(obj) ```` This affects both shallow and deep copying. My idea for a fix is to add property objects to the list of "atomic" objects in the copy module. These already include types like functions and type objects. I also added property objects to the unit tests test_copy_atomic and test_deepcopy_atomic. This is my first PR, and it's highly likely I've made some mistake, so please be kind :) https://bugs.python.org/issue38293

Allow shallow and deep copying of property objects + unit tests

Loading status checks…

c766497

the-knights-who-say-ni added the CLA signed label Sep 27, 2019

bedevere-bot added the awaiting review label Sep 27, 2019

eamanu reviewed Sep 28, 2019

View changes

brandtbucher added the type-enhancement label Sep 29, 2019

brandtbucher suggested changes Sep 29, 2019

View changes

bedevere-bot added awaiting core review and removed awaiting review labels Sep 29, 2019

📜🤖 Added by blurb_it.

Verified

This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.

GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits

Loading status checks…

26fe8e7

brandtbucher approved these changes Sep 29, 2019

View changes

gvanrossum reviewed Oct 23, 2019

View changes

gvanrossum approved these changes Oct 23, 2019

View changes

bedevere-bot added awaiting merge and removed awaiting core review labels Oct 23, 2019

csabella added needs backport to 3.7 needs backport to 3.8 🤖 automerge labels Jan 12, 2020

bedevere-bot removed the awaiting merge label Jan 12, 2020

miss-islington self-assigned this Jan 12, 2020

bedevere-bot removed the needs backport to 3.7 label Jan 12, 2020

csabella removed the needs backport to 3.8 label Jan 12, 2020

csabella added the needs backport to 3.8 label Jan 12, 2020

bedevere-bot removed the needs backport to 3.8 label Jan 12, 2020

python / cpython

bpo-38293: Allow shallow and deep copying of property objects #16438

bpo-38293: Allow shallow and deep copying of property objects #16438

GudniNatan commented Sep 27, 2019 •

edited by miss-islington

eamanu left a comment

brandtbucher left a comment •

edited

This comment has been minimized.

GudniNatan commented Sep 29, 2019

gvanrossum left a comment

This comment has been minimized.

arigo commented Oct 23, 2019

This comment has been minimized.

gvanrossum commented Oct 23, 2019

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

bedevere-bot commented Jan 12, 2020

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

bedevere-bot commented Jan 12, 2020

This comment has been minimized.

brandtbucher commented Jan 12, 2020

python / cpython

Sponsor python/cpython

Join GitHub today

bpo-38293: Allow shallow and deep copying of property objects #16438

bpo-38293: Allow shallow and deep copying of property objects #16438

Conversation

GudniNatan commented Sep 27, 2019 • edited by miss-islington

eamanu left a comment

brandtbucher left a comment • edited

This comment has been minimized.

GudniNatan commented Sep 29, 2019

gvanrossum left a comment

This comment has been minimized.

arigo commented Oct 23, 2019

This comment has been minimized.

gvanrossum commented Oct 23, 2019

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

bedevere-bot commented Jan 12, 2020

This comment has been minimized.

miss-islington commented Jan 12, 2020

This comment has been minimized.

bedevere-bot commented Jan 12, 2020

This comment has been minimized.

brandtbucher commented Jan 12, 2020

GudniNatan commented Sep 27, 2019 •

edited by miss-islington

brandtbucher left a comment •

edited