Python: Making SKFunction.RunThread an outer class

[mkarle]

Motivation and Context

It's better to avoid nested classes in Python, so in SKFunction, I move RunThread out of the class. RunThread is needed because we need to wait for the result of an async function that's being run in an existing event loop, and subclassing Thread seems to be the best way to do that. This also adds python-test.yml to the github workflows.

Description

• Move RunThread out of SKFunction since flat hierarchy is more desirable than nested hierarchy in python.
• Tried using a local method instead, but to no avail
  • I had to invoke _invoke_semantic_async with await or async.run, but that could only be done within a Thread because the jupyter notebook spawns an event loop. To wait for the result, you need to use join() but it doesn't return the result, so the local method needed an output parameter that I could mutate. That is, I passed in a list and modified the list inside the method I pass into the Thread. I didn't like that
  • I tried creating a task inside the existing event loop but couldn't wait for the result without a callback
• Ran tests, pre-commit hooks, and notebooks

For python-test.yml:

• In this PR, the checks were run because python files were changed.
• In this test PR: https://github.com/mkarle/semantic-kernel/pull/2, the python tests were skipped because files in the python directory were not changed.

Contribution Checklist

• [x] The code builds clean without any errors or warnings
• [x] The PR follows SK Contribution Guidelines (https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
• [x] The code follows the .NET coding conventions (https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions) verified with dotnet format
• [x] All unit tests pass, and I have added new tests where possible
• [x] I didn't break anyone :smile:

[dluc]

could we simplify further, avoiding the class?

def runThread(code):
    x = threading.Thread(target=code)
    x.start
    x.join
    return x.result

if loop and loop.is_running():
    if self.is_semantic:
        return runThread(self._invoke_semantic_async(context, settings))
    else:
        return runThread(self._invoke_semantic_async(context))
else:
    if self.is_semantic:
        return asyncio.run(self._invoke_semantic_async(context, settings))

[mkarle]

Unfortunately, that won't work.
With x = threading.Thread(target=self._invoke_semantic_async), the async function will never be awaited. But Thread doesn't have a result property, so even with x = threading.Thread(target=asyncio.run, args=(self._invoke_semantic_async,)), we can't get the result.

This works though. You can pass in a list that you modify.

def runCode(code, result):
       result.append(asyncio.run(code)
def runThread(code):
       result = []
       thread = threading.Thread(runCode(code, result)) 
       thread.start()
       thread.join()
       return result[0]

if loop and loop.is_running():
    if self.is_semantic:
       return runThread(self._invoke_semantic_async(context, settings)

Do you like that better?

[dluc]

@mkarle yes that looks better, using methods and not having an extra class is nicer.

nitpicking, is it possible to write thread.start().join() or somehow combine the 2 lines?

anyhow, pls make this change, love it!

[mkarle]

start and join both return None, so I don't think I can combine those lines