graphql-code-generator icon indicating copy to clipboard operation
graphql-code-generator copied to clipboard

Python (not python-operations, at least yet)

Open Andrew-Talley opened this issue 4 years ago • 35 comments

Since @dotansimha suggested in #3712 that @seansfkelley open a WIP PR, and I'm currently working on some python stuff, I figured this might be useful. Right now, I'm working on something much simpler than the python operations – just the schema types. Mostly this is just to help me familiarize myself with the codebase before working on Python operations.

There's still a few things that need to get done:

  • [X] Interfaces (this might be tricky)

  • [x] More tests for comments

  • [x] Input types

  • [x] Custom scalars

  • [x] Config options (I'm not even sure how many of those I've broken already)

  • [x] Add all typescript tests

  • [x] Figure out min versions of Python that will support this,

  • [ ] (Long-term?) Try to add options to permit supporting earlier versions of Python (e.g., typing came in Python 3.5 – maybe we add an option to remove all types, so that if we do add something that uses Python GraphQL libraries, we can support versions before 3.5. Also – literal types are only supported in 3.8, so we could allow an option not to use a literal typename).

Something brief for the maintainers: I exported isStringValueNode() and made DeclarationBlock._config a protected (nor private) variable. The latter was a more important move, but I figured I'd flag both of those now. Also, if anyone feels so inclined, I'd welcome any comments on the code, it's definitely not in great shape yet.

Andrew-Talley avatar May 31 '20 22:05 Andrew-Talley

A couple updates – @seansfkelley, if you have the time to test this, I'd greatly appreciate it. Thank you again for the advice you gave a couple weeks back. I ended up using native typing as opposed to pydantic just so the base version wouldn't have any python dependencies. That being said, I really like the idea of adding pydantic as another plugin akin to TypeGraphQL or something. That way, people can still use it, but can also avoid adding that dependency if they want.

@dotansimha, I think this package is almost ready to go, just two things:

  1. I'm just not sure what the process is for actually adding a package. I'd love a way to put it in some kind of beta first – either a badge labeling it as beta, only adding it to alpha versions, waiting to add it to the documentation so people don't necessarily discover it, etc. Just let me know which (if any of these) sounds good. Alternatively, if you'd rather avoid any type of beta, if I can get the documentation working (see below why I haven't been able to), that'll give me a better interface to generate and test the resulting code and get that last bit of confidence in this.
  2. I have run into a couple issues with getting the documentation ready – all the dynamic import statements in website/src/components/live-demo/plugins.ts throw a Module not found error. I'm guessing this is because of the new live-demo format (which, by the way, looks amazing). I think it's just an issue on my end, and I doubt there's anything wrong with the Python-specific docs since I got them working with the 1.15.3 version (and every import is throwing an error), but I just want to note that before anything goes horribly wrong. Similarly, if you ran into that and know what I need to fix, let me know, and I'd love to get that working.

Andrew-Talley avatar Jun 14 '20 23:06 Andrew-Talley

Hey @Andrew-Talley, just took a look -- making good progress! I'm not sure how many of these issues you're already aware of, so here's a list of everything I observed and some responses to your comments.

I'm down with the idea of class Scalars as a namespace. There's a couple issues though:

  1. any = Any both clobbers the global method any (admittedly, unlikely to matter) and results in an unidiomatic type definition.
  2. I'm not sure if mypy is going to be happy about Scalars.Foo functioning as a type alias: https://github.com/python/mypy/issues/6941. In my repos, I use the singleton-union workaround to force the heuristic to understand this is a type alias in a class-as-namespace: String = Union[str].
  3. DateTime is any instead of datetime or str. Not sure what the right thing to do here is, but any is surprising.

If it's easy to add Pydantic after the fact, I think it's reasonable to not bake it in. I figure an option like baseClass could be added that would add a parent to every generated class would be sufficient for my use-cases.

The biggest issue is that of operation-aware types though. As you are probably aware, the Typescript generator follows the pattern of "generate complete type definitions in one file, then Pick/Partial to define operations types". It appears you've started down the same path here with Python, but I don't see any way to mimic the second half of that -- the pick/partial behavior -- in a way that wouldn't ultimately mislead any typecheckers (be they runtime -- Pydantic -- or compile time -- mypy).

This is why my initial approach was to do python and python-operations simultaneously. The plan was that python would just be scalars, enums, input types and utils (e.g. the Pydantic superclass), and python-operations would do all the heavily lifting of generating all the other types. I'm also a little concerned (though, admittedly, have not thought through all the eventualities) that your current strategy (pick/partial-dependent) and my proposal (per-operations-types only) are mutually exclusive, which is going to force us to choose rather than allow consumers more flexibility.

All that said, I think this is a good first pass -- one could deserialize a GraphQL payload into these types and simply be aware that certain fields, though type-annotated, don't actually have values at runtime based on the shape of the query.

Thanks for continuing to work on this! And as always, I am happy to take looks and provide feedback along the way. I still don't foresee a good opportunity to get my hands dirty with implementation, unfortunately, as we've backburnered a lot of the code that would benefit from this feature.

seansfkelley avatar Jun 15 '20 20:06 seansfkelley

Hey @seansfkelley – thanks for the feedback! I think I agree with basically everything you said. As far as any = Any goes – I'm definitely not a fan of it, just left it in place for now. That's a good catch with the Scalars.Foo – I hadn't actually tested it with mypy, just checked if python would run it and the IDE would autosuggest, so I'll have to look back into that. And as for DateTime being any, I think that'll be a config option so people can choose what type it'll be.

For everything else – I agree with most of what you said for the differences between python-operations and typescript-operations. I think the structure you mentioned in your initial message (thank you again for that) is what I'll need to go with, so I do think python-operations will have to do a lot more heavy-lifting (since I at least haven't found an equivalent to Pick in python). I just chose to do a base python plugin first a) just to get familiar with the code base, b) to have an easier way to get scalar types, and c) (like you mentioned) so that people could use this initially as long as they remained cognizant they didn't have access to every part of the graph, and d) to mirror the typescript structure to make it easier for people new to this repo. As far as being mutually exclusive – I think I basically agree, and I think yours is the only one that's possible. I'm also hoping I'll be able to get some code like this:

class usersQuery:
  ...
  userId: UserDefinitedType.userId
  ...

but I agree that each query will need a number of its own classes.

Thank you again for taking a look – I'll let you know when I've addressed the issues you've raised, and then again (probably in a bit more than a month) once I have a working python-operations plugin.

Andrew-Talley avatar Jun 15 '20 21:06 Andrew-Talley

Right now, I know of two issues:

  1. The reasons the build is failing is a bit of a chicken-and-egg problem: since @graphql-codegen/python package doesn't exist on npm yet, the website can't build with it, and so the build fails. So, to the reviewers, since I think some of you have dealt with adding a new plugin, I'll defer to you all on this.
  2. Right now, to permit circular references, it refers to different types in Python implicitly (i.e., someType: Optional["UserType"]). However, this can lead to a scope issue when a single type has a) a field with the name of some user-defined field, and b) another field that references that user-defined type. For a more concrete example, this schema can't be validated by mypy:
type Query {
  Tweet: Tweet
  Tweets: [Tweet] # <- Here, python ends up thinking "Tweet" refers to Query.Tweet, which could be the incorrect type
}

I'm working to fix the second one now, so if anyone is looking at starting to review this, there may be a decent amount changing soon.

andrewdtalley-3 avatar Jun 25 '20 02:06 andrewdtalley-3

Just pushed something that fixes the second problem above. Now, each user-defined type has an alias (__GQL_CODEGEN_[UserType]__) that is referenced by other types. It does hamper the readability of the code, but the only other solution I could come up with (defining each attribute outside the scope of the class) hurt type-checking even worse. It might be possible to add a config option to remove this behavior, and enabling that would drop support for fields with the same name as some other type, but that certainly doesn't seem pressing.

At this point, this package is in as good of a state as I think I can get it in for now. I'd gladly welcome any feedback or reviews.

Andrew-Talley avatar Jun 27 '20 18:06 Andrew-Talley

This is really awesome! Thank you so much @Andrew-Talley ! and thanks @seansfkelley for the productive review.

To be honest, I'm not highly familiar with Python, so I have to trust you with the plugin output. I will review the code and usage of the codegen internal to see that we are not missing something, or to check if something could be reused.

@Andrew-Talley is there something else that is missing in order to get this merged? (you can ignore the failing canary CI task, it's fine, i'll make sure to fix that before merging)

Regarding the next steps and chores regarding adding a new package, we have this docs that might help you: https://github.com/dotansimha/graphql-code-generator/blob/master/website/docs/custom-codegen/contributing.md#8-integration

Basically, add some docs and explanation for usage, how to integrate this with Python apps, and just make sure to have a config.ts file that exports the configuration TS interface of the plugins, and I'll take care of the rest to make sure to integrate the config structure with the YAML validation.

@seansfkelley I saw you added a base types package for python as well, what do you think we should do with that? How can we merge you work together? Maybe we can start with this one, and then in the future put some effort on making python-operations?

dotansimha avatar Jun 30 '20 09:06 dotansimha

I should have some time to take a look at this tomorrow, and I can try to answer your questions. I haven't seen the output for the latest version of the plugin yet, either.

seansfkelley avatar Jul 01 '20 01:07 seansfkelley

@dotansimha I don't think there are any loose ends. I just pushed the one or two things I thought might come up once it was pushed, so I think it should be good to go. I was able to get the website up and running locally, and the docs are a slightly modified version of the typescript docs, so hopefully that's enough. I did add an @example decorator to the config.ts file about how someone could use it in python code, but it looks like docusaurus doesn't permit that by default. Not sure if that's an easy change, but it's in there if you know a way to easily add it.

For the rest of the code – two things to flag:

  1. The prettier command after yarn generate:examples now has errors because it doesn't know how to prettify a python file. I tried to add a glob pattern to ignore python files, but that just made things worst, so just figured I'd mention that here.
  2. One broader thing I found with a non-Typescript language is that using a custom declaration block can be a little awkward. There were methods I probably could've avoided redeclaring if I could override the default declaration block. My thought was to make a declaration block factory method on BaseTypesVisitor. Then, a method like EnumTypeDefinition could call this.newDeclarationBlock() instead of new DeclarationBlock(). Certainly not a huge deal, and if it's not easy to change or would lead to a bunch of other complications, it was easy to get around it with copy and paste. Either way, figured it was worth mentioning, and if you'd be open to a PR with that change, I'd be happy to try to implement it (this just didn't feel like the place for it).

More generally, I was blown away with how easy it was to extend this package. I certainly don't want to degrade the code quality, so please feel free to point out any code you think could be improved and either correct it yourself or ask me to.

@seansfkelley I have basically the same questions that @dotansimha had, whenever you get a chance to look things over. I'm happy to keep going with the project, or if you want to try to implement python-operations, I'm happy to support you wherever I can. When I last looked at your fork, I wasn't quite able to figure out how much progress you had made. So, if you would like me to keep going and make a python-operations plugin, let me know how far you brought the code, and if you think it would be easier for me to build it just using the lessons you learned, or try to keep going where you left off.

Andrew-Talley avatar Jul 01 '20 04:07 Andrew-Talley

tl;dr: Output looks good save for a recommendation to use 3.7-style postponed annotations in place of forward refs.

For anyone following along at home, you can get yarn to install despite the chicken-and-egg problem noted above by just removing the python package as a dependency from the website package.

Onto the real stuff!

So just to refresh my understanding, this summary I provided a few comments ago still seems to be the case:

All that said, I think this is a good first pass -- one could deserialize a GraphQL payload into these types and simply be aware that certain fields, though type-annotated, don't actually have values at runtime based on the shape of the query.

With that in mind, I think there's no need to do python-operations yet, and I certainly wouldn't block this PR on it. This is valuable as-is.

When I last looked at your fork, I wasn't quite able to figure out how much progress you had made.

Yeah, sorry about that. It was never really intended to see the light of day in that state and I just dumped it into a PR. Part of the problem was something you point out:

One broader thing I found with a non-Typescript language is that using a custom declaration block can be a little awkward.

The utilities as currently written definitely favor Flow/TypeScript patterns, which is to be expected, but I also found it makes Python generation a little clunky. Ah well.

Now, each user-defined type has an alias (__GQL_CODEGEN_[UserType]__) that is referenced by other types.

Two things about this:

  • You may have to use the Union trick like you did with Scalars to get mypy to be happy. (My hacked setup right now is a little too messy to be able to check this with my tests in a reasonable amount of time, otherwise I would.)
  • Are you okay only supporting only Python 3.7+? I would strongly recommend it, because then you can use PEP 563's postponed annotations to remove all these type aliases and string forward-refs. You just have to prepend from __future__ import annotations. (Note: this may interact badly with the add plugin used in prepend mode, because __future__ imports MUST come at the top of the file, but I'm okay punting on that problem for now.)

I saw you added a base types package for python as well, what do you think we should do with that?

@dotansimha We should prefer this one. @Andrew-Talley and I talked a bit above about future directions for this package, especially when python-operations comes about. We agreed that the code generated by the python package is likely to change quite a bit when that happens, but it doesn't make this first step any less useful.

So, if you would like me to keep going and make a python-operations plugin...

Well yes, I would love that. :) I'm under the impression that you don't need it for your use-case, though?

... let me know how far you brought the code, and if you think it would be easier for me to build it just using the lessons you learned, or try to keep going where you left off.

I haven't reviewed your implementation at all (leaving that to @dotansimha), just the results it produces. I can't say how compatible our approaches are, but my gut says your implementation is going to be much more mature and my implementation is better scrapped for parts and ideas. I'm happy to provide any learnings if/when you take on python-operations!

seansfkelley avatar Jul 01 '20 18:07 seansfkelley

@seansfkelley Thank you again for the continued feedback on this. To respond quickly to what you've mentioned:

As far as 3.7 is concerned, I like the idea of adding that as on option. Looking at the documentation, though, I wasn't able to see the concrete benefit of using future annotations over forward string refs beyond cleaner code. Certainly, that's a worthy goal, so I'm happy to add it. But, as far as the default goes, my thought would be to support more versions, just so new/inexperienced users are less likely to run into issues. However, if I just missed some obvious, larger benefit, please let me know, and then I think making it the default would be the right call.

For the union trick – Python apparently does have type aliases, they just don't work as part of a class. Haven't figured out why, but there you go. When I ran the output using mypy, it seemed fine with those global aliases, even though it did throw errors for the scalar stuff you mentioned earlier. That being said, if it works differently for you, let me know and I'll take another look.

As far as python-operations goes, I actually started working on this specifically to later support python-operations, so I'm happy to keep working on it. I may also start working on a graphene generator since that matches the structure of the visitor a bit better, so if you (or the one other person who seems to be following this thread) has a preference between those, let me know. I also saw someone else had started one (#2516) to enthusiastic response, so I may reach out to them to see if they want me to start working on it.

Andrew-Talley avatar Jul 05 '20 19:07 Andrew-Talley

As far as 3.7 is concerned, I like the idea of adding that as on option. Looking at the documentation, though, I wasn't able to see the concrete benefit of using future annotations over forward string refs beyond cleaner code.

I figured it would help get rid of __GQL_CODEGEN_[UserType]__, but upon further thought, postponed annotations shouldn't enable anything more than non-string forward refs. So I guess my actual question is: what's the use of these aliases, and why can't you just have a string forward ref directly to the generated type?

...Python apparently does have type aliases, they just don't work as part of a class

Ah, yes, that makes sense. It guess it can't easily differentiate between a proper class field and a class acting as a namespace, but it has no such difficulty with global-scope aliases.

As far as python-operations goes, I actually started working on this specifically to later support python-operations, so I'm happy to keep working on it.

I figured as much. For my part, if/when I have time to contribute, it makes no difference to me whether this is merged or on a branch. As it stands, I can't quite use it yet anyway (I need python-operations and ideally Pydantic support), so I'm not in a rush. Whatever you need to make this a real thing as expediently as possible is fine by me.

I may also start working on a graphene generator since that matches the structure of the visitor a bit better, so if you... [have] a preference between those, let me know.

I do, actually, and it would be for python-operations. In my case, the schema is generated from Graphene classes, so I don't need to go that direction. :) But I'm not the one doing the work, so don't let me dictate which way you should go (unless you need a tiebreaker).

seansfkelley avatar Jul 06 '20 16:07 seansfkelley

@seansfkelley The reason for the alias is separate from the cyclical refrences – essentially, if you have this schema:

type Query {
  Tweet: Tweet
  Tweets: [Tweet!]!
}

it'll generate this code:

class Query:
  Tweet: Optional[Tweet]
  Tweets: List[Tweet] # Problem line

The problem is, the type List[Tweet] now refers to the type Query.Tweet, so here, it would say that the elements in the array could be null (which would be a problem).

As for future plugins, I'll probably take a quick look at graphene this weekend to see how easy it would be, but I'll definitely come back around to python-operations. In the meantime, @dotansimha, let me know if there's anything I can do to help get this finalized.

Andrew-Talley avatar Jul 08 '20 03:07 Andrew-Talley

Argggh, that's what happens when you mix your type and value namespaces together, Python! I can confirm that it does weird stuff in PyCharm. Here's a screenshot of it giving me type information for that example:

image

With the extra bonus:

image

(I defined Tweet = int for testing.)

This happens with and without the = None (I had been wondering if having annotations without values would behave differently from specifying a value, but it appears not).

seansfkelley avatar Jul 08 '20 17:07 seansfkelley

Why not to use dataclasses from py3.7? Or dataclassy to lower it to py3.6? From how the current code looks like it should only take an import and an annotation. Inline type hints will already prevent from going lower than py3.5, so the interpreter version shouldn't pose much of an issue anyway. dataclasses will add init, asdict, and some other methods, which might be a nice addition

upcFrost avatar Sep 29 '20 16:09 upcFrost

I suggested Pydantic because:

  • it supports aliasing, making snake <> camel conversions trivial
  • it supports faux immutability, which is useful for query/mutation results
  • it does runtime type validation, unlike dataclasses
  • it has all the key features of dataclasses, too

That said, I like the idea of an opt-in Pydantic mode and using dataclasses as the default.

seansfkelley avatar Sep 29 '20 21:09 seansfkelley

That said, I like the idea of an opt-in Pydantic mode and using dataclasses as the default

Would be nice imo. I'm just a bit concerned about possible performance issues (Pydantic had some serious pitfalls in the past), but that's case-specific. Also, IIRC there's a pydantic.dataclasses.dataclass decorator which can be used as a drop-in replacement for the dataclasses.dataclass, which should make it much easier to support both options.

upcFrost avatar Sep 30 '20 07:09 upcFrost

@Andrew-Talley This looks really cool! Is there anything we can help you to get this merged?

ardatan avatar Feb 09 '21 20:02 ardatan

@Andrew-Talley When do you think you'll be ready for merging? I really could need python support right now.

maaft avatar Mar 15 '21 11:03 maaft

I want to pick this PR up and also do operations with dataclasses as a start. Currently installation is failing with this message
image

How to continue this development?

unimonkiez avatar May 05 '21 12:05 unimonkiez

🦋 Changeset detected

Latest commit: b237c787d28414f5b67dabe5001894144064470c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/python Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar May 05 '21 13:05 changeset-bot[bot]

I fixed merge conflicts and aligned the versions so yarn install should work fine. @unimonkiez

ardatan avatar May 05 '21 13:05 ardatan

I fixed merge conflicts and aligned the versions so yarn install should work fine. @unimonkiez

Thanks, will pick up on it hopefully operations will be ok.
BTW, I saw that you made Scalars a class but then I'm not seeing type hint correcly.. Changed to simple types and it works correctly (using Pylint on vscode) Before:

class Scalars:
  ID = Union[str]
  String = Union[str]
  Boolean = Union[bool]
  Int = Union[int]
  Float = Union[float]

class Agc:
  __typename: Optional[Literal["AGC"]]
  target_bits: Scalars.Float
  starting_gain: Scalars.Float
  step_size: Scalars.Float

After

ScalarID = str
ScalarString = str
ScalarBoolean = bool
ScalarInt = int
ScalarFloat = float

class Agc:
  __typename: Optional[Literal["AGC"]]
  target_bits: ScalarFloat
  starting_gain: ScalarFloat
  step_size: ScalarFloat

Any input on that or I'll just go with that?

unimonkiez avatar May 05 '21 14:05 unimonkiez

Hey, sorry I haven't been responding to this thread. @unimonkiez if you want to work on this, please do. A few notes though:

  1. As far as changing it from ScalarID vs. Scalar.ID, I think the original way had worked for me, but if your solution works on more setups, then that seems like the obvious choice. I would also assume it's unlikely
  2. A larger problem – one thing that discouraged my progress here is that I haven't found clients that serialize their data into the kind of format that's assumed here. This is what @seansfkelley had mentioned earlier. For example, with graphene, everything is a series of dictionaries that are accessible by key. JS libraries normally do the same, but TypeScript is a lot more flexible than Python's typing system, and I didn't find a way to add Python hints based on the specific key (e.g., to have a number type for obj['field1'] but a string for obj['field2']). I also don't believe dataclasses will solve that, but Python isn't my area of expertise so I could be wrong. I've been trying to come up with a solution, but I'm still empty handed. The best solution might be to skip straight to python-operations and give up on type hinting. Obviously not ideal, but it would still be nice to separate graphql files from python files for the same reasons you want to separate HTML from CSS. You could also potentially generate the code to serialize it yourself (potentially on an operation-by-operation level). But, that would introduce a runtime overhead, which seems different than a lot of the other packages. Hopefully, there's an option that's the best of both worlds, I'm just not sure what that is.

That second part being said – if people still think this would be useful, then it's worth continuing to pursue, and I might be able to pick something like this back up later. I just think that limitation is worth highlighting again, since this package wouldn't be that helpful without operations, and I'm not sure how well that could use typehints.

Andrew-Talley avatar May 05 '21 18:05 Andrew-Talley

Hey, sorry I haven't been responding to this thread. @unimonkiez if you want to work on this, please do. A few notes though:

  1. As far as changing it from ScalarID vs. Scalar.ID, I think the original way had worked for me, but if your solution works on more setups, then that seems like the obvious choice. I would also assume it's unlikely
  2. A larger problem – one thing that discouraged my progress here is that I haven't found clients that serialize their data into the kind of format that's assumed here. This is what @seansfkelley had mentioned earlier. For example, with graphene, everything is a series of dictionaries that are accessible by key. JS libraries normally do the same, but TypeScript is a lot more flexible than Python's typing system, and I didn't find a way to add Python hints based on the specific key (e.g., to have a number type for obj['field1'] but a string for obj['field2']). I also don't believe dataclasses will solve that, but Python isn't my area of expertise so I could be wrong. I've been trying to come up with a solution, but I'm still empty handed. The best solution might be to skip straight to python-operations and give up on type hinting. Obviously not ideal, but it would still be nice to separate graphql files from python files for the same reasons you want to separate HTML from CSS. You could also potentially generate the code to serialize it yourself (potentially on an operation-by-operation level). But, that would introduce a runtime overhead, which seems different than a lot of the other packages. Hopefully, there's an option that's the best of both worlds, I'm just not sure what that is.

That second part being said – if people still think this would be useful, then it's worth continuing to pursue, and I might be able to pick something like this back up later. I just think that limitation is worth highlighting again, since this package wouldn't be that helpful without operations, and I'm not sure how well that could use typehints.

  1. Cool.
  2. It is possible to serialize the data into a class instead of a dictionary.

unimonkiez avatar May 09 '21 07:05 unimonkiez

Thanks for the great work from @Andrew-Talley and @unimonkiez !

add Python hints based on the specific key (e.g., to have a number type for obj['field1'] but a string for obj['field2']).

TypedDict can do that. It's availble since Python 3.8 but can be used on older versions via typing_extensions

whtsky avatar May 18 '21 13:05 whtsky

I also don't believe dataclasses will solve that

Dataclasses do solve type hinting actually, similar to any other class with type hints on its fields. Dataclasses however have some drawbacks for code generation, namely the inheritance. Like, if your dataclass has fields with default values, and you want to make a subclass, then all fields of the subclass should have default parameters. It is linked to how the __init__ method is composed by the dataclass decorator. GraphQL does not support inheritance though, so it's not a big issue in this case.

Dataclasses are also pretty slow and trying to inject __slots__ into them can lead to a decent amount of "fun" in some specific cases, but well, writing a high-perf service in python is not the most trivial thing anyway.

On the other hand, it is much easier to work with dataclasses using libs like dacite, which together with dataclasses.asdict method pretty much solves serialisation problems. So, tbh using dataclasses might be a good idea

upcFrost avatar May 18 '21 14:05 upcFrost

On the other hand, it is much easier to work with dataclasses using libs like dacite, which together with dataclasses.asdict method pretty much solves serialisation problems. So, tbh using dataclasses might be a good idea

That exactly what I did in this PR, any help to merge this will be great.

Also, found a bug in this PR.
Will open an issue soon.

Edit:
So you can't open issue on forks..
Anyway, when defining a mutation with an input, this will make it's class to be used before it is defined, and when importing that code python will throw an error like so:

class MutationTestArgs:
  File "~/dev/python_graphql_operations_codegen_demo/python_graphql_operations_codegen_demo/graphql_generated/types_generated.py", line 37, in MutationTestArgs
    data: TestDataInput
NameError: name 'TestDataInput' is not defined

Reproduce:

  • Clone this.
  • Checkout branch 'bug-schema'.
  • Install deps
    • poetry run start
    • yarn install
    • cd server && yarn install
  • Copy <THIS_PROJECT>/packages/plugins/python/python/* to <DEMO_PROJECT>/node_modules/@graphql-codegen/python (after build).
  • Run server cd server && yarn start
  • Run codegen yarn codegen
  • Run python poetry run start

Workaround for now:
manually move the input's class to be before it is used, not ideal.

unimonkiez avatar May 27 '21 00:05 unimonkiez

data: TestDataInput

Put quotes around the class name, it's a legal syntax for dataclass forward references for type hints

@dataclass
class A:
    b: 'B'

@dataclass
class B:
    x: int

There is a drawback that if you're going to use reflection to instantiate fields based on the field type, it will fail (check dataclasses.fields value type in the debugger). But for type hinting it is ok to use, and most IDEs will follow it

Edit: It is the same syntax you use for factory method hinting, when you can not refer the class from itself from the method signature

upcFrost avatar May 27 '21 09:05 upcFrost

Put quotes around the class name, it's a legal syntax for dataclass forward references for type hints

Cool, for now I added a sed command to fix that in my python project, I might pick up this bug myself in the future.

BTW, if anyone wants to use this (and operations) in their's python project right now and not wait until the PR is merged - I've added steps to do so here.

unimonkiez avatar May 27 '21 15:05 unimonkiez

Hi,

we also made another fork of @unimonkiez fork where we fixed some bugs. Further changes:

  • optional async client
  • replaced async subscription client with a synchonous version
  • no near-operation-file preset needed - you'll get one generated file
  • one client class with easy access to all defined graphql operations

codegen.yml example:

schema: http://localhost:8080/graphql
documents: "./query/operations/**/*.graphql"
generates:
  ./query/types.py:
    plugins:
      - python
    config:
      skipDocumentValidation: true
      useIndexSignature: true
  ./query/operations.py:
    config:
      generateAsync: false
    plugins:
      - add:
          content: "import platform_api.query.types as Types"
          placement: "prepend"
      - python-operations:
          schema: "unused"
          schemaSubscriptions: "unused"
          headerName: "unused"
          headerValue: "unused"
config:
  skipDocumentsValidation: true

The codegen surley needs some heavy polishing, but it works like a charm so far.

DENKweit avatar Jun 30 '21 14:06 DENKweit