There doesn't seem to be a way to override is_type_of on objects about graphql-core HOT 9 CLOSED

Hi @Cito , we might need to reopen this!
Unfortunately, in cases such as using an inherited __typename property, basic demangling is not enough.
Let's imagine I have the following (Relay inspired) Node and subclass:

class Node:
    @property
    def __typename(self):
        return self.__class__.__name__

class A(Node):
    ....

Obviously _A__typename will not be found, and we will ignore _Node__typename.

Something like this (instead of just using a simple getattr()) would help instead:

def get_typename(value):
    for cls in value.__class__.__mro__:
        __typename = getattr(value, f"_{cls.__name__}__typename", None)
        if __typename:
            return __typename
    return None

What do you think?

from graphql-core.

This is implemented now in the master branch and will be available in the next beta release.

from graphql-core.

HI @ezyang, thanks for reporting this issue.

is_type_of is actually a method of the GraphQL type which checks whether the given object is of that type. What you're probably looking for is the __typename attribute which you can set on objects to specify their GraphQL type as a string and which is checked by default_resolve_type_fn. This is the way it's done also in GraphQL.js which graphql-core-next copies.

However, I see that that graphql-core-next currently checks __typename only on objects that are returned as dicts, not on (data)class instances like in your case.

It would certainly be a good idea and very simple to extend graphql-core-next to check for __typename attributes on non-dict objects as well - you would simply set __typename = 'Repository' on your Repository class. Unfortunately, there is an issue here in that Python applies name mangling to attributes starting with two underscores (a feature intended to be used for fake "private" attributes that is frowned upon but unfortunately still exists in Python 3).

So probably we should use the special name __typename__ instead (these names do not underly name mangling). It's not nice, since these special names are actually reserved for Python, and we deviate a bit from the GraphQL API, but it's the only obvious solution that comes to my mind just now. What do you think?

from graphql-core.

A special name like __typename__ seems fine to me, as long as it's prominently documented.

Note that it's not impossible to demangle it, if you really wanted to. But I think it's probably better here to just hew to Pythonic conventions.

from graphql-core.

I also considered demangling and immediately dismissed the idea. But now after sleeping over it, I am thinking maybe it is not so bad an idea after all. The rule for the mangling - prefixing with the class name- is well documened and not implementation specific, and the class name can be easily inspected. That would allow us to be completely compatible to GraphQL.js and consistent between dicts and class based objects.

Another option would be check both attributes (__typename and __typename__), but that would go against the Zen of Python.

from graphql-core.

So I decided to go with the demangling. This will be available in the next version (released soon).

Please reopen if you have any better ideas.

from graphql-core.

Version 1.0,2 with this fix has now been released on PyPI.

from graphql-core.

Good point @Hellzed. Will add your A(Node) example and maybe and a more complicated one with multiple inheritance to the test suite. Your suggested code will probably solve this.

from graphql-core.

Version 3.1.0b1 with this fix has been released now.

from graphql-core.