Skip to content

feat(dialogs): Incorporate DialogDependencies #2219

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexrecuenco
Copy link
Contributor

@alexrecuenco alexrecuenco commented Mar 23, 2025

Fixes #2218

Description

Dialog dependencies is a feature in

To maintain feature parity, and because this functionality is extremely useful this PR brings it to botbuilder-python.

Specific Changes

Tecnically in the botbuilder-js and dotnet implementations dialogs that are found to conflict get stored with a different ID, but I am sure that would likely be a breaking change (and IMO a very counter-intuitive way of working).

If you are adding dialog dependencies and they conflict, the logic as it was would be retained.

Note that, although by making it a @runtime_checkable Protocol with DialogDependencies one can check isinstance(object, DialogDependencies), the recommendation from the python docs is:

"check against a runtime-checkable protocol can be surprisingly slow" — python typing

For that reason we use hasattr and callable instead.

Also, note that protocol is avaialble from python 3.8 onwards, which is already the minimum version for this library.

The purpose of using Iterable instead of List is just to keep it generic. Since python has the concepts of tuple, List, etc and to keep it as a structural type.

Testing

  • Tests with coverage is added
  • pylint was run against the code, (although note that the pre-commit used uses a dependency that doesn't work in python 3.12, updating to repo: https://github.com/pylint-dev/pylint would be nice
  • black was run against the code (note that some libraries are actually not properly formatted according to black!)

@@ -14,6 +14,9 @@
from .dialog import Dialog
from .dialog_state import DialogState

if TYPE_CHECKING:
from .dialog_context import DialogContext
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is necessary for the -> "DialogContext" used below, otherwise intellisense has a hard time understanding that the object returned is a DialogContext. It has no runtime impact because of TYPE_CHECKING being False at runtime

Dialog dependencies is a feature in

- [botbuilder-js](https://github.com/microsoft/botbuilder-js/blob/87d0ad1a889a38396e761f10614c78a4526590d1/libraries/botbuilder-dialogs/src/dialogSet.ts#L169),
- and in [botbuilder-dotnet](https://github.com/microsoft/botbuilder-dotnet/blob/bc17a5db2c717fec0d4c109d884bea88d29f75cb/libraries/Microsoft.Bot.Builder.Dialogs/DialogSet.cs#L147).

To maintain feature parity, and because this functionality is extremely
useful this PR brings it to `botbuilder-python`.

Tecnically in the `botbuilder-js` and dotnet implementations dialogs that
are found to conflict get stored with a different ID, but I am sure that
would likely be a breaking change (and IMO a very counter-intuitive way
of working).

If you are adding dialog dependencies and they conflict, the logic as it
was would be retained.

Note that, although by making it a `@runtime_checkable` Protocol with
DialogDependencies one can check `isinstance(object, DialogDependencies)`,
the recommendation from the python docs is:

"check against a runtime-checkable protocol can be surprisingly slow"
— [python typing](https://docs.python.org/3/library/typing.html#typing.Protocol)

For that reason we use `hasattr` and `callable` instead.

Also, note that protocol is avaialble from python 3.8 onwards,
which is already the minimum version for this library.

The purpose of using Iterable instead of List is just to keep it generic.
Since python has the concepts of `tuple`, `List`, etc and to keep it as a structural type.
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.

Add DialogDependencies, working towards feature parity with botbuilder-js and botbuilder-dotnet
1 participant