Skip to content
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.

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

Support customizing factory constructors names from union types #33

Open
davidmigloz opened this issue Nov 2, 2023 · 2 comments
Open

Comments

@davidmigloz
Copy link
Contributor

davidmigloz commented Nov 2, 2023

I had in my mind a feature request to allow to specify custom factory names, which (together with the already implemented titles) would be the ultimate piece for full customization support.

My idea was to use Specification Extensions to define the factory name in the schema (e.g. using something like x-factoryName):

function_call:
  title: ChatCompletionFunctionCall
  description: Controls how the model calls functions.
  oneOf:
    - type: string
      title: ChatCompletionFunctionCallMode
      x-factoryName: mode
      enum: [none, auto]
    - $ref: "#/components/schemas/ChatCompletionFunctionCallOption"
ChatCompletionFunctionCallOption:
  type: object
  x-factoryName: functionCallOption
  properties:
    name:
      type: string
      description: The name of the function to call.
  required:
    - name

Originally posted by @davidmigloz in #32 (comment)

          > My idea was to use [Specification Extensions](https://swagger.io/specification/#specification-extensions) to define the factory name in the schema (e.g. using something like `x-factoryName`):

Yea this package does not introduce any sort of package specific customization like that. The idea was to just be compatible with any API schema out of the box, without introducing any extra dependency or custom definitions.

@davidmigloz alternatively, we can do it at the code generation level with an onUnionConstructorName function that would allow a user to override the constructor of a specific union. Feel free to open an issue and we can move that chat there.

Thanks again for the fast implementation. lgtm!

Will cut a new release with these changes.

Originally posted by @walsha2 in #32 (comment)

@davidmigloz
Copy link
Contributor Author

I see your point, it may be better to do it at the code generation level to keep the spec pure and clean.
onUnionConstructorName sounds like a good idea, in line with the other functions.

@walsha2
Copy link
Contributor

walsha2 commented Nov 2, 2023

Sounds good. I'll get that in, I could actually use such a function myself!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants