-
Notifications
You must be signed in to change notification settings - Fork 132
Treat pandas as an optional dependency (#489) #536
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
|
Any chance to get this reviewed? |
Thanks! @FBruzzesi I realize this change wasn't pushed properly to my branch. Updated it as well as rebased to resolve the |
|
Hello, I'm just wondering if it's possible to get any movement on this? The large pandas dependency is a blocker for my project. |
88098c7 to
439fbd4
Compare
|
Any chance of getting this reviewed? |
|
Is there any way to accelerate this PR? The underlaying issue was opened almost a year ago, and this PR nearing half a year and the issue is still quite blocking for being able to use databricks-sql-python at all. If there is any way for me to contribute, lmk. |
|
@gs11 it appears that there are conflicts. If I had a guess, it would be the lock file needs to be regenerated. |
src/databricks/sql/client.py
Outdated
| kwargs = {**kwargs, **access_token_kv} | ||
|
|
||
| self.disable_pandas = kwargs.get("_disable_pandas", False) | ||
| self.disable_pandas = False if pandas is None else kwargs.get("_disable_pandas", False) |
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.
| self.disable_pandas = False if pandas is None else kwargs.get("_disable_pandas", False) | |
| self.disable_pandas = True if pandas is None else kwargs.get("_disable_pandas", False) |
I had that backwards. If panda is not installed, it should be disabled.
|
Pandas is also imported here:
|
|
I imagine they would want a warning message similar to the one for pyarrow: databricks-sql-python/src/databricks/sql/client.py Lines 81 to 86 in 4b7df5b
|
df0d1ad to
e905f74
Compare
Thanks, updated. |
Signed-off-by: Gustav Sinder <gustav.sinder@gmail.com>
|
Are there any tests in the CI run without pandas installed? If not, it would be important to add for future stability. |
I can't comment on the effectiveness but there's
|
That's helpful. Did you check the CI files to see if there is a test run without optional dependencies installed? |
What type of PR is this?
Description
This naive PR changes pandas to be an optional dependency - just like pyarrow.
Tests fail but seemingly due to unrelated things.
How is this tested?
Related Tickets & Documents
#489