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

Pydantic improvements #372

Merged
merged 87 commits into from
Aug 19, 2024
Merged

Pydantic improvements #372

merged 87 commits into from
Aug 19, 2024

Conversation

ZergLev
Copy link
Collaborator

@ZergLev ZergLev commented Jul 12, 2024

Description:

  • TLDR: All classes in /chatsky/pipeline/ are rewritten in Pydantic, some changes to class hierarchy and the classes themselves. Changed tutorials with regards to new class signatures.
  • Classes PipelineComponent, Pipeline and ComponentExtraHandler are now subclasses of Pydantic's BaseModel, massively reducing code bloat in their initialization and that of their children (such as Service).
  • Also, classes Service, ServiceGroup and Actor are now subclasses of PipelineComponent. No functionality is removed in ServiceGroup. All tutorials and tests pass like before, with a few changes.
  • Reasoning for changing class hierarchy: Before this, an instance of Actor would be called from within a Service with a special flag, which seemed like a workaround, and now it can be called directly as an instance of PipelineComponent, instead of heavily relying on the Service class. You could say that this slightly reduces the code's complexity. In fact, it also binds Actor to PipelineComponent, instead of it being independent, making code look more cohesive.
  • TypeBuilders have been removed, because Pydantic can already deal with them effectively, giving type hints for IDE like before. Different input types are still handled like before, except nested Service declaration is removed. (Service class doesn't accept Service as it's handler anymore) Same with nested ComponentExtraHandlers.
  • For some reason Extra Handlers in Services would work even if the Service itself was turned off via start_condition. That was a bug. Now this condition is checked in PipelineComponent instead. I assume Actor must always have it as True.

Checklist

  • Removed from_script() and other redundant methods.
  • Changed components into pre-services and post-services throughout tutorials. (at Pipeline initialization)
  • Got tests to pass.
  • I have performed a self-review of the changes
  • Updated Pipeline's API and tutorials

To Consider

  • Add more tests if necessary
  • Update the remaining API reference / tutorials / guides
  • Search for more references to changed entities in the codebase

ZergLev added 30 commits July 10, 2024 07:32
…s to from_script() before, that's not handled right now)
…uted_fields from Pipeline, they're not working as intended
chatsky/pipeline/pipeline/component.py Outdated Show resolved Hide resolved
chatsky/pipeline/pipeline/component.py Outdated Show resolved Hide resolved
chatsky/pipeline/pipeline/component.py Outdated Show resolved Hide resolved
chatsky/pipeline/pipeline/component.py Outdated Show resolved Hide resolved
chatsky/pipeline/pipeline/component.py Outdated Show resolved Hide resolved
chatsky/pipeline/service/extra.py Outdated Show resolved Hide resolved
chatsky/pipeline/service/extra.py Outdated Show resolved Hide resolved
chatsky/pipeline/service/group.py Show resolved Hide resolved
chatsky/pipeline/service/group.py Show resolved Hide resolved
chatsky/pipeline/service/service.py Outdated Show resolved Hide resolved
@RLKRo RLKRo merged commit 385f0a5 into dev Aug 19, 2024
17 checks passed
@RLKRo RLKRo mentioned this pull request Sep 7, 2024
@RLKRo RLKRo mentioned this pull request Sep 27, 2024
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 this pull request may close these issues.

2 participants