Skip to content

Update object type docs #921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 17, 2019
Merged

Update object type docs #921

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50321c3
Update object type docs
jkimbo Mar 16, 2019
38db79c
Merge branch 'master' into update-object-type-docs-resolver
ekampf Mar 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 6 additions & 42 deletions docs/types/objecttypes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ so the first argument to the resolver method ``self`` (or ``root``) need
not be an actual instance of the ``ObjectType``.

If an explicit resolver is not defined on the ``ObjectType`` then Graphene will
attempt to use a property with the same name on the object that is passed to the
``ObjectType``.
attempt to use a property with the same name on the object or dict that is
passed to the ``ObjectType``.

.. code:: python

Expand All @@ -70,54 +70,18 @@ attempt to use a property with the same name on the object that is passed to the

class Query(graphene.ObjectType):
me = graphene.Field(Person)
best_friend = graphene.Field(Person)

def resolve_me(_, info):
# returns an object that represents a Person
return get_human(name='Luke Skywalker')

If you are passing a dict instead of an object to your ``ObjectType`` you can
change the default resolver in the ``Meta`` class like this:

.. code:: python

import graphene
from graphene.types.resolver import dict_resolver

class Person(graphene.ObjectType):
class Meta:
default_resolver = dict_resolver

first_name = graphene.String()
last_name = graphene.String()

class Query(graphene.ObjectType):
me = graphene.Field(Person)

def resolve_me(_, info):
def resolve_best_friend(_, info):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh I just opened an issue on your other PR about putting self into each method as per python convention, and also its used in other examples in the docs. But now here I see this place in the docs it is also set to be _ previously. Having it both ways in the docs feels not-consistent.

In the case of this particular PR your addition here matches the convention in this file of not having self so I'm fine with just merging this in and maybe we can figure out the whole self-vs-underscore thing across the docs separately...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And elsewhere it is root, info, **args

Consistent would be fantastic

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dan98765 so the reason I don't use self when defining resolvers is because self means a specific thing in python (the instance of the current class) but in Graphene the first argument to the resolver is not an instance of the current class. Actually resolvers work like static functions and the first param is whatever the parent resolver returned.

Using self in the past has led to people being confused about what the value actually is and assuming it's the current instance (which is not unreasonable) so I've taken to naming the value as _ when it's at the top of the tree (because if there is no root value then it's always None) or calling it a name that refers to what I'm expecting to receive. For example:

class User(graphene.ObjectType):
	name = graphene.String()

	def resolve_name(user, info):  # <-- the first argument will be a Django model instance
		return user.name

class Query(graphene.ObjectType):
	user_by_id = graphene.Field(User, id=graphene.Int())
	
	def resolve_user_by_id(_, info, id):  # <-- no root value so the first argument is always None
		return DjangoUser.objects.get(id=id)

I agree that we should standardise on something and actually thinking about it it makes sense to standardise on root for the top level Query resolvers (rather than _) so I'm happy to change the documentation to reflect that. I would also strongly recommend not using self for any resolver documentation because I think it ends up being confusing. Does that make sense @dan98765 @ProjectCheshire ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, @jkimbo! I agree that we should not use self unless the it actually does represent the instance of the current class in the standard python way.

Actually resolvers work like static functions and the first param is whatever the parent resolver returned.

Could resolvers be decorated with @staticmethod to make this more explicit? Or does that break something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if you can add @staticmethod to resolvers actually but I don't see why it wouldn't work. It would get quite tedious to have to do it everywhere though.

return {
"first_name": "Luke",
"last_name": "Skywalker",
"first_name": "R2",
"last_name": "D2",
}

Or you can change the default resolver globally by calling ``set_default_resolver``
before executing a query.

.. code:: python

import graphene
from graphene.types.resolver import dict_resolver, set_default_resolver

set_default_resolver(dict_resolver)

schema = graphene.Schema(query=Query)
result = schema.execute('''
query {
me {
firstName
}
}
''')


Resolvers with arguments
~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down