Skip to content
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

Simplify functions array #18

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

thecapacity
Copy link

Some Syntactical Sugar seems like it would make maintenance easier

Syntactic Sugar to make maintenance easier
update to match new way to auto-build `functions` list in `functions.py`
Add some error checking
@interstellarninja
Copy link
Collaborator

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

@CNX-Inc
Copy link

CNX-Inc commented May 13, 2024

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
Copy link
Collaborator

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
Copy link

CNX-Inc commented May 29, 2024

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.

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.

3 participants