classification
Title: descriptor __set_name__ feature broken for dataclass descriptor fields
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: Ricyteach, eric.smith, miss-islington
Priority: normal Keywords: patch

Created on 2018-03-26 02:25 by Ricyteach, last changed 2018-03-26 18:14 by eric.smith. This issue is now closed.

Files
File name Uploaded Description Edit
broken__set_name__.py Ricyteach, 2018-03-26 02:25 raises AttributeError on accessing `descriptor_object.name` attribute
Pull Requests
URL Status Linked Edit
PR 6260 merged eric.smith, 2018-03-26 16:34
PR 6261 merged miss-islington, 2018-03-26 17:29
Messages (16)
msg314438 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 02:25
Summary: The descriptor `__set_name__` functionality (introduced in Python 3.6) does not seem to be working correctly for `dataclass.Field` objects with a default pointing to a descriptor. I have attached a file demonstrating the trouble.

Details: If I set a `dataclass` class object field to a `dataclass.field` with a descriptor object for the `default` argument, the descriptor's `__set_name__` method is not called during initialization. This is unexpected because descriptors themselves seem to work pretty much flawlessly, otherwise. 

(Bravo on that by the way! Working descriptors isn't mentioned at all in the PEP as a feature but I was very pleased to see them working!!)

System details:
Python 3.7b02
Windows 10
PyCharm Community Edition

btw this is my first ever Python bug report; I hope I did a good job.
msg314446 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 10:28
That's a tough one. Because C.d is not set to a descriptor at type creation time (it's set to a Field object), the __set_name__ behavior is never invoked. It's when the @dataclass decorator is called that C.d is set to D().

https://docs.python.org/3/reference/datamodel.html#creating-the-class-object

I'm not sure it's possible to work around this without duplicating some of the type creation code, and even then I'm not convinced it's doable.
msg314447 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 10:47
I suppose I could, when overwriting the class member, check for inspect.ismethoddescriptor and call __set_name__ myself.
msg314452 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 13:49
hmmm... if I check the C.d class attribute it seems to return the
descriptor instance object (not a field object) before any C instances have
been created. i guess this is just a part of how the dataclass
implementation works.

i didn't realize there's nothing "special" going on with descriptors here-
the descriptors "just work" by virtue of being set to the class attribute
at creation time. interesting.

maybe because of this descriptors should short-circuit the field creation
process entirely? that would be a shame though. having the option of
auto-including a descriptor in the class repr turns out to be very useful
and i'm already playing around with it in a project.

one idea: what if it there were a keyword argument to mark a field as a
descriptor, allowing tje descriptor to be set at type creation? this would
need to disallow init=True, i think. and setting a field default to a
descriptor class would then raise a type error.

---
Ricky.

"I've never met a Kentucky man who wasn't either thinking about going home
or actually going home." - Happy Chandler

On Mon, Mar 26, 2018 at 6:47 AM, Eric V. Smith <report@bugs.python.org>
wrote:

>
> Eric V. Smith <eric@trueblade.com> added the comment:
>
> I suppose I could, when overwriting the class member, check for
> inspect.ismethoddescriptor and call __set_name__ myself.
>
> ----------
> components: +Library (Lib)
> versions: +Python 3.8
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue33141>
> _______________________________________
>
msg314458 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 15:20
Nick Coghlan suggested (on python-dev) to have Field implement __set_name__ and call through to the default value, if it exists.

That approach seems to work fine. I'll generate a PR with tests sometime before today's release.
msg314459 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 15:46
Sounds like a relatively easy solution then. Hopefully it makes the beta 3 so I can use it immediately- thanks!
msg314460 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 16:30
I suppose one downside of that solution is __set_name__ will be called for every Field whether or not it is need. Probably can't be helped without major complications.
msg314463 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 16:45
I don't expect there to be too much usage of Field objects, so I'm not so worried about it.

If you can, try out the code in the PR. Thanks.
msg314466 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 16:57
Eric, looking at the PR; note that if you do this for the __set_name__ check:

    if inspect.ismethoddescriptor(self.default):

...an object like the one below will not get its __set_name__ called, even though PEP 487 says it should:

class D:
	def __set_name__(self, o, n):
		self.name = n

class C:
	d: int = D()

if __name__ == "__main__":
	print(f"C.d.name = {C.d.name}") # __set_name__ was called
	assert inspect.ismethoddescriptor(C.d) # Error
msg314467 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 17:00
Yes, I noticed the same thing. I'll remove the test.
msg314468 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 17:03
Yeah and I think lines 2709-2712 of TestDescriptors also needs removed.
msg314469 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 17:05
I've updated the PR.

I left those lines in, and added a different test. After all, it does need to work with real descriptors.
msg314470 - (view) Author: Rick Teachey (Ricyteach) Date: 2018-03-26 17:09
Looks great to me. Thanks!
msg314471 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 17:29
New changeset de7a2f04d6b9427d568fcb43b6f512f9b4c4bd84 by Eric V. Smith in branch 'master':
bpo-33141: Have dataclasses.Field pass through __set_name__ to any default argument. (GH-6260)
https://github.com/python/cpython/commit/de7a2f04d6b9427d568fcb43b6f512f9b4c4bd84
msg314473 - (view) Author: miss-islington (miss-islington) Date: 2018-03-26 17:55
New changeset c6147acd2ce5fa9e344f179b539f3b21b9ae1a6d by Miss Islington (bot) in branch '3.7':
bpo-33141: Have dataclasses.Field pass through __set_name__ to any default argument. (GH-6260)
https://github.com/python/cpython/commit/c6147acd2ce5fa9e344f179b539f3b21b9ae1a6d
msg314474 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2018-03-26 18:14
Thanks for the bug report.
History
Date User Action Args
2018-03-26 18:14:56eric.smithsetstatus: open -> closed
resolution: fixed
messages: + msg314474

stage: patch review -> resolved
2018-03-26 17:55:15miss-islingtonsetnosy: + miss-islington
messages: + msg314473
2018-03-26 17:29:42miss-islingtonsetpull_requests: + pull_request5987
2018-03-26 17:29:18eric.smithsetmessages: + msg314471
2018-03-26 17:09:15Ricyteachsetmessages: + msg314470
2018-03-26 17:05:19eric.smithsetmessages: + msg314469
2018-03-26 17:03:21Ricyteachsetmessages: + msg314468
2018-03-26 17:00:40eric.smithsetmessages: + msg314467
2018-03-26 16:57:12Ricyteachsetmessages: + msg314466
2018-03-26 16:45:38eric.smithsetmessages: + msg314463
2018-03-26 16:34:22eric.smithsetkeywords: + patch
stage: patch review
pull_requests: + pull_request5984
2018-03-26 16:30:48Ricyteachsetmessages: + msg314460
2018-03-26 15:46:04Ricyteachsetmessages: + msg314459
2018-03-26 15:20:48eric.smithsetmessages: + msg314458
2018-03-26 13:49:01Ricyteachsetmessages: + msg314452
2018-03-26 10:47:31eric.smithsetmessages: + msg314447
components: + Library (Lib)
versions: + Python 3.8
2018-03-26 10:28:57eric.smithsetmessages: + msg314446
2018-03-26 03:53:12rhettingersetassignee: eric.smith
2018-03-26 02:25:49Ricyteachcreate