.Net: Change IKernelPlugin to be abstract class KernelPlugin

[markwallace-microsoft]

Motivation and Context

Changing IKernelPlugin to be abstract class KernelPluginBase so that we have the flexibility to extend that we get from abstract classes.

Description

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
• All unit tests pass, and I have added new tests where possible
• I didn't break anyone 😄

[stephentoub]

What's the reason for this change?

If we want to convert IKernelPlugin to be a base class instead, I'd suggest a few changes:

• Just have it be called KernelPlugin rather than ReadOnlyKernelPlugin.
• Change the existing KernelPlugin to be named something more like ComposedKernelPlugin, DynamicKernelPlugin, ManualKernelPlugin, etc. The number of folks that will need to consume / work with the base class is much, much larger than the number of folks that need to manually build up a new KernelPlugin from individual functions, so we should use the "good" name for the base class.
• Any extension method we currently have in Abstractions (and possibly in Core, depending on what it is) around an IKernelPlugin should move to being an instance member of the base KernelPlugin.

[stephentoub]

Change the existing KernelPlugin to be named something more like ComposedKernelPlugin, DynamicKernelPlugin, ManualKernelPlugin, etc. The number of folks that will need to consume / work with the base class is much, much larger than the number of folks that need to manually build up a new KernelPlugin from individual functions, so we should use the "good" name for the base class.

Actually, the current KernelPlugin (the mutable one) probably doesn't even need to be public. You just have a KernelPluginFactory method that takes a name and an IEnumerable<KernelFunction> and it returns an internal type we can name whatever we want. Then we just use KernelPlugin as the good name for the abstract base class.

[markwallace-microsoft]

What's the reason for this change?
Changing IKernelPlugin to be abstract class ReadOnlyKernelPlugin so that we have the flexibility to extend that we get from abstract classes.

• Just have it be called KernelPlugin rather than ReadOnlyKernelPlugin.
  OK I will make this change

• Change the existing KernelPlugin to be named something more like ComposedKernelPlugin, DynamicKernelPlugin, ManualKernelPlugin, etc.
  Need to think about this name.

• Any extension method we currently have in Abstractions (and possibly in Core, depending on what it is) around an IKernelPlugin should move to being an instance member of the base KernelPlugin.
  Will make this change

[stephentoub]

Change the existing KernelPlugin to be named something more like ComposedKernelPlugin, DynamicKernelPlugin, ManualKernelPlugin, etc. The number of folks that will need to consume / work with the base class is much, much larger than the number of folks that need to manually build up a new KernelPlugin from individual functions, so we should use the "good" name for the base class.

Actually, the current KernelPlugin (the mutable one) probably doesn't even need to be public. You just have a KernelPluginFactory method that takes a name and an IEnumerable<KernelFunction> and it returns an internal type we can name whatever we want. Then we just use KernelPlugin as the good name for the abstract base class.

I like this for the additional reason that it means b someone can't downcast to the mutable type and start adding functions to some random plugin.

[markwallace-microsoft]

Change the existing KernelPlugin to be named something more like ComposedKernelPlugin, DynamicKernelPlugin, ManualKernelPlugin, etc. The number of folks that will need to consume / work with the base class is much, much larger than the number of folks that need to manually build up a new KernelPlugin from individual functions, so we should use the "good" name for the base class.

Actually, the current KernelPlugin (the mutable one) probably doesn't even need to be public. You just have a KernelPluginFactory method that takes a name and an IEnumerable<KernelFunction> and it returns an internal type we can name whatever we want. Then we just use KernelPlugin as the good name for the abstract base class.

For tooling scenarios we want to allow a developer to add functions to the Plugin. This is what Diego does in his .Net Interactive tool. So if we don't make the implementation that allows this public, do we add extension methods for AddFunction and AddFunctions that only work when the implementation is ours?

[stephentoub]

For tooling scenarios we want to allow a developer to add functions to the Plugin. This is what Diego does in his .Net Interactive tool

I see. Add functions one by one? In that case, yes, someone would need a mutable implementation, though it doesn't necessarily need to be in the core. With most of the functionality in the base class, someone who needs that interactivity could write the class which would be pretty short, right?