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

feat: publicly export SqlBackend #10365

Open
1 task done
NickCrews opened this issue Oct 24, 2024 · 5 comments
Open
1 task done

feat: publicly export SqlBackend #10365

NickCrews opened this issue Oct 24, 2024 · 5 comments
Labels
feature Features or general enhancements

Comments

@NickCrews
Copy link
Contributor

NickCrews commented Oct 24, 2024

I have this custom function, which I would like to type annotate correctly:

import ibis
from ibis.backends.sql import SQLBackend


def insert_people(conn: SQLBackend, new_people: ibis.Table):
    ...
    # ibis.BaseBackend doesn't have an .insert() method
    conn.insert("people", new_people)
    ....

I don't want to have to reach all the way into ibis.backends.sql to get the import. Can we expose this at top-level, similar to ibis.BaseBackend?

Code of Conduct

  • I agree to follow this project's Code of Conduct
@NickCrews NickCrews added the feature Features or general enhancements label Oct 24, 2024
@cpcloud
Copy link
Member

cpcloud commented Nov 2, 2024

I'm not sure what the issue is here.

Are you typing this so often that making it a public API is actually worth the maintenance cost?

@gforsyth
Copy link
Member

gforsyth commented Nov 2, 2024

I'm more inclined to remove BaseBackend from the top-level.

@NickCrews
Copy link
Contributor Author

Here is how often I am using it in a private repo:

image

Specifically for my usages, for SqlBackend, I am trying to type "something that has a .insert(), .create_table(), .create_view(), and .raw_sql() methods". For BaseBackend, I am trying to type "something that has a .table(), .read_csv(), and .read_parquet() methods".

So I guess what I could do if I wanted typing would be to make my own little Protocols and use those, and then just add in #noqa's at the the boundaries of where I start using them.

Or, SqlBackend probably isn't going to change very often, so I would be fine absorbing the personal maintenance cost of adjusting my code as needed if the internals of Ibisi change.

But, isn't the selling point of ibis that it is easy to change between backends? We even test for that I think, right, like that .insert() works the same across backends? Maybe we should even publish our own Protocol that defines exactly what API we want to make public/stable, because perhaps that could be different from what BaseBackend and SqlBackend support?

Is the maintenance cost you are thinking of like "oh, we added a param to all our backends, and now we need to remember to update in this other place too"? Or is it that we don't want to gloss over the fact that different backends DO have differences, like some are going to have read_parquet() support, and others aren't? I think at least for this last concern we could address this by being very conservative with what methods we include in the Protocol, eg just create_table(), create_view(), drop_table(), etc, and none of the .read_* methods. Something could be better than nothing, if someone wants to write a type hint for "something that has a read_parqet() method", then they can do that themselves with Protocols.

@gforsyth
Copy link
Member

gforsyth commented Nov 4, 2024

Hey @NickCrews -- there's going to be assumptions here on my part (WARNING):

I think that the "average" user of Ibis (whatever that means) is less likely to be writing functions and classes that operate on arbitrary backend objects -- that's a relatively advanced thing to do and more akin to writing a library on top of Ibis (which, of course, you are doing).

Including generic objects like BaseBackend and SQLBackend as top-level imports is polluting the namespace a bit, since a new user tab-completing on ibis. is only going to be confused by those objects showing up.

Is having it as ibis.SQLBackend important for the keystrokes? Or is it more that you want a consistent location to import these things from? If the latter, maybe we'd be better served by a sort of proxy typing module (or here is where some protocol object might be exposed) to collect the various base types easier/more consistent access?

@NickCrews
Copy link
Contributor Author

@gforsyth you hit the nail on the head, I just want consistency/stability. Thanks!

Other things I type all the time that may want to go in this module:

  • ibis.expr.types stuff, eg StringValue (and less often, the specific shapes of StringScalar and String column)
  • less often, but sometimes ibis.expr.datatypes.DataType
  • Deferred, idk maybe this wants to move out of the top-level namespace
  • these Backend classes
  • Table, Column, Value could get left at top-level? Or we be consistent and everything goes in eg ibis.types

In order of preference:

  • ibis.types (import ibis.types as it) (shortest)
  • ibis.typing (import ibis.typing as it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Features or general enhancements
Projects
Status: backlog
Development

No branches or pull requests

3 participants