deeppavlov / chatsky

Chatsky is a free and open-source software stack for creating chatbots, released under the terms of Apache License 2.0.
https://deeppavlov.github.io/chatsky/
Apache License 2.0
32 stars 9 forks source link

Feat/core rework #381

Closed RLKRo closed 2 months ago

RLKRo commented 2 months ago

Description

Breaking Changes

Features

Fixes

Documentation

Devel

Checklist

To Consider

kudep commented 2 months ago

Short review:

RLKRo commented 2 months ago
  1. run is a method that runs it with the set interface. When using telegram interface pipeline.run is used as well. I could add comments explaining that pipeline.run starts up its interface which is CLI by default.
  2. OK, I like dst.This. The only problem is that it would make less sense for LOCAL and GLOBAL than Current.
  3. I'm assuming you mean in tutorials because order can be any. That suggestion makes sense.
  4. If you're referring to the removed option to pass text as a first positional argument, that has been re-added here.
  5. This is part of service update.
  6. I did not understand this comment. Did you mean that Transition.cnd should never be a literal True or False? I don't see a reason to forbid this, but it is needed to have True by default.
  7. I agree. I have a task to add a subclass of BaseProcessing that hides these parts making it easy to define a Processing that modifies response. I can implement that earlier and insert into the tutorial.
  8. My thought process was that slot tutorial is not the first one user is going to see, so it's fine to introduce these convenient (hopefully) abbreviations at this point. But I can remove them.