-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
build(dependencies)📦: Add numpydoc to dependencies in pyproject.toml #10736
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
base: main
Are you sure you want to change the base?
Conversation
- Include numpydoc for documentation generation in utils.py.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@krrishdholakia @ishaan-jaff apologies for not tagging you guys earlier. Please let me know if there’s anything you believe I’ll need to do. |
LGTM |
Hey @ericmjl since numpydoc is not required to make a .completion() request, I'm hesitant to add it as a package requirement (more bloat). Is there a reason it needs to be a required dependency? |
Good point, @krrishdholakia! It becomes an issue when we use the function_to_dict function to convert a function into a tool schema. Is this planned to be supported long-term? If not, I’m happy to close the PR to save on deps and maintenance work. |
What would you want here @ericmjl This seems similar to how integrations might have their own dependencies but it's not part of the core requirements. |
I’m happy to follow your lead here! If I were to suggest, making it an optional dependency might be what I would go for. |
That sounds reasonable! Let's do that. Feel free to update your PR to reflect that. Happy to merge it in. |
…try.lock and pyproject.toml - Set several packages as optional in poetry.lock. - Add 'extra == "utils"' marker to various packages in poetry.lock. - Update numpydoc dependency to be optional in pyproject.toml. - Add 'utils' extra section in pyproject.toml with numpydoc.
@krrishdholakia can I check, are you ok with the current arrangement of deps, or would you prefer a different one? |
Resolves #10735.