-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[Install] Rework optional imports #767
[Install] Rework optional imports #767
Conversation
Tagging @ryanpeach @hudson-ai and @Harsha-Nori , since I'm sure you will all have suggestions :-) So far, I've just done some work on adding a |
setup.py
Outdated
@@ -52,8 +52,8 @@ def find_version(*file_paths): | |||
], | |||
extras_require={ | |||
"docs": ["ipython", "numpydoc", "sphinx_rtd_theme", "sphinx", "nbsphinx"], | |||
"schemas": ["jsonschema", "pydantic"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next up would be server
and all of the various supported models (each in their own extras).
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #767 +/- ##
==========================================
- Coverage 65.69% 62.49% -3.21%
==========================================
Files 55 55
Lines 4064 4058 -6
==========================================
- Hits 2670 2536 -134
- Misses 1394 1522 +128 ☔ View full report in Codecov by Sentry. |
@ryanpeach and @Harsha-Nori , I've updated this in response to comments, and also extracted I have tried to give |
Thanks @riedgar-ms! I also ran into this when I dropped the openai dep. LGTM |
Much better! |
Tbh pydantic might become a useful required dependency. Ever thought of just keeping it? |
I'm keeping it required for now. It is something which might be worth keeping generally. |
LGTM too. I'm also ok with Pydantic being a required import per @ryanpeach's suggestion |
Sorry for being late to the party! Looks good. Thanks @riedgar-ms |
Trying to make optional imports behave more uniformly, with support in
setup.py
for lots of possible extras.