NousResearch / Hermes-Function-Calling

MIT License
475 stars 74 forks source link

Simplify functions array #18

Open thecapacity opened 3 months ago

thecapacity commented 3 months ago

Some Syntactical Sugar seems like it would make maintenance easier

interstellarninja commented 2 months ago

Could you please briefly explain what lines 312-321 do?

CNX-Inc commented 2 months ago

Absolutely:

The real work is via the inspect module .isfunction(...) and the getattr(...)__.module__ items.

functions = [func for func in dir(__main__) if inspect.isfunction(getattr(__main__, func)) and getattr(__main__, func).__module__ == '__main__']

This creates a list of all items in the file (via getattr(...)) and then uses inspect.isfunction(...) to filter down to only functions that are defined in the file, i.e. if a variable were defined in the file's namespace it would be filtered out and the contents would just be functions, which should match the previously (manually) managed function = [ ] array.

By way of some initial checking the previously manually updated functions = [ ] list was renamed to old_functions and then a try: ... except: ... block was used to confirm that the automatically calculated values are == the manually updated ones. I presume if the logic holds the old_functions and manual update wouldn't be necessary, making adding (or deleting) functions in the functions.py file easier.

Note, if someone created helper functions that they wanted to use internally within the file but not expose 'externally' to the language model this might be incorrect. However, the guidance for the file (including in the README.md) suggests that any functions added to functions.py should be listed in the array, so this seems like a minimal concern.

interstellarninja commented 2 months ago

understood -- it makes sense to automatically detect functions in the functions.py file there's a caveat to this approach though since we only want to convert functions with "tool" decorator.

it may be safe to assume that only functions passed as tools to LLM will be added to this file and we could use this method as default

how about deprecating the old method with manually added list of functions into old_functions and automatically creating a list of functions defined within functions?

i'll merge once that is implemented. thanks.

CNX-Inc commented 1 month ago

It doesn't look like there's an 'easy' way to test for a function being decorated by a specific decorator without modifying the decorator itself...

i.e. the [second] solution here works:

    return hasattr(func, '__wrapped__') or func.__name__ not in globals()

But it would return true for any decorated function, not just those marked @tool - so it may be more possible to get specific but I couldn't figure out an easy way.

If you wanted to at least narrow the function scope you could modify:

functions = [func for func in dir(__main__) if inspect.isfunction(getattr(__main__, func)) and getattr(__main__, func).__module__ == '__main__']

To include an and is_decorated(...) component.

also I agree with you on removing the old_functions section - I didn't have an easy way to test and wanted to make sure to leave some checks for initial assurance.