On 11/30/2017 3:35 PM, Carl Meyer wrote:
On 11/29/2017 05:02 PM, Guido van Rossum wrote:
I tried to look up the discussion but didn't find much except that you flagged this as an issue. To repeat, your concern is that isdataclass() applies to *instances*, not classes, which is how Eric has designed it, but you worry that either through the name or just because people don't read the docs it will be confusing. What do you suppose we do? I think making it work for classes as well as for instances would cause another category of bugs (confusion between cases where a class is needed vs. an instance abound in other situations -- we don't want to add to that). Maybe it should raise TypeError when passed a class (unless its metaclass is a dataclass)? Maybe it should be renamed to isdataclassinstance()? That's a mouthful, but I don't know how common the need to call this is, and people who call it a lot can define their own shorter alias.
Yeah, I didn't propose a specific fix because I think there are several options (all mentioned in this thread already), and I don't really have strong feelings about them:
- Keep the existing function and name, let it handle either classes or
instances. I agree that this is probably not the best option available, though IMO it's still marginally better than the status quo).
- Punt the problem by removing the function; don't add it to the public
API at all until we have demonstrated demand.
- Rename it to "is_dataclass_instance" (and maybe also keep a separate
"is_dataclass" for testing classes directly). (Then there's also the choice about raising TypeError vs just returning False if a function is given the wrong type; I think TypeError is better.)
In that case, you can spell "is_dataclass_instance":
def isdataclass_instance(obj): dataclasses.fields(obj) # raises TypeError for non-dataclass # classes or instances if not isinstance(obj, type): raise TypeError('not an instance') return True
Since this is easy enough to do in your own code, and I still don't see a use case, I'll just add a note to the PEP and omit delete isdataclass().
Plus, you can decide for yourself how to deal with the question of returning true for classes or instances or both.