[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

Thanks for the bug report.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[miss-islington]

miss-islington  added the comment:


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


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[miss-islington]

Change by miss-islington :


--
pull_requests: +5987

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

Looks great to me. Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

Yeah and I think lines 2709-2712 of TestDescriptors also needs removed.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

Yes, I noticed the same thing. I'll remove the test.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Change by Eric V. Smith :


--
keywords: +patch
pull_requests: +5984
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

Sounds like a relatively easy solution then. Hopefully it makes the beta 3 so I 
can use it immediately- thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

Rick Teachey  added the comment:

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 
wrote:

>
> Eric V. Smith  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 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Eric V. Smith]

Eric V. Smith  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Raymond Hettinger]

Change by Raymond Hettinger :


--
assignee:  -> eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue33141] descriptor __set_name__ feature broken for dataclass descriptor fields

[Rick Teachey]

New submission from Rick Teachey :

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.

--
files: broken__set_name__.py
messages: 314438
nosy: Ricyteach, eric.smith
priority: normal
severity: normal
status: open
title: descriptor __set_name__ feature broken for dataclass descriptor fields
type: behavior
versions: Python 3.7
Added file: https://bugs.python.org/file47499/broken__set_name__.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com