-
Notifications
You must be signed in to change notification settings - Fork 590
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
Update to handle change in sql_validate return type #2256
Conversation
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.
Thanks @jclay. Do we need to require a higher version of pymapd after this change?
Also, would be good to have a test, and an entry in the release notes. Thanks!
3590315
to
fcb4429
Compare
|
@datapythonista I have updated the CI suite to use the latest pymapd released today. I will open a PR on the feedstock repo as well. Note that this was only affecting OmnisciDB v5.3.0 users, so while Ibis had good test coverage for this already, we were testing against v5.2.2 so it was not showing up in the tests. I have added coverage of I'm not sure if it's related to the pymapd version bump, but I am noticing a drastic increase in the time it takes for conda to resolve the dependency constraints. Once |
|
mmm for some reason it is raising this error: not sure why it is changing the result size: |
|
@jclay, You can investigate the problems with |
|
@jclay is this something we still want to get merged? |
|
I think we will want to get this merged at some point given it fixes the |
|
Hi @jclay, any update on this? Spoke to Eric K and he mentioned it was causing major headaches for a prospect. |
This is to accommodate the changes made to the OmniSci client in heavyai/pymapd#325