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

refactor: standardize list_tables signature #2877

Closed
datapythonista opened this issue Jul 24, 2021 · 1 comment · Fixed by #4793
Closed

refactor: standardize list_tables signature #2877

datapythonista opened this issue Jul 24, 2021 · 1 comment · Fixed by #4793
Labels
backends Issues related to all backends refactor Issues or PRs related to refactoring the codebase

Comments

@datapythonista
Copy link
Contributor

All backends seem to provide list_tables, but with a variety of signatures.

I propose to standardize all them to the same, so things are simpler and clearer, and code can be reused from one backend to another easier. My preferred signature would be:

def list_tables(self, like: str = None) -> List[str]:

We can add a FutureWarning for Ibis 2.0, if other parameters are used. I'll add **kwargs, and if not empty, a warning will be shown, and in 3.0 we can remove 3.0. I'll also deprecate list_tables in Database, in favor of the Client one.

Let me know if anyone disagrees, or any comment.

The list of current signatures is next:

ibis/backends/clickhouse/client.py:    def list_tables(self, like=None, database=None):
ibis/backends/mysql/client.py:    def list_tables(self, like=None, database=None, schema=None):
ibis/backends/csv/__init__.py:    def list_tables(self, path=None):
ibis/backends/parquet/__init__.py:    def list_tables(self, path=None):
ibis/backends/dask/client.py:    def list_tables(self, like: str = None) -> List[str]:
ibis/backends/hdf5/__init__.py:    def list_tables(self, path=None):
ibis/backends/spark/client.py:    def list_tables(self, like=None, database=None):
ibis/backends/postgres/client.py:    def list_tables(self, like=None, database=None, schema=None):
ibis/backends/impala/kudu_support.py:    def list_tables(self, filter=''):
ibis/backends/impala/client.py:    def list_tables(self, like=None, database=None):
ibis/backends/sqlite/client.py:    def list_tables(self, like=None, database=None, schema=None):
ibis/backends/base/file/__init__.py:    def list_tables(self, path=None):
ibis/backends/base/file/__init__.py:    def list_tables(self, path=None):
ibis/backends/base/sql/alchemy/client.py:    def list_tables(
ibis/backends/base/sql/alchemy/database.py:    def list_tables(self, like=None):
ibis/backends/base/sql/alchemy/database.py:    def list_tables(self, like=None, schema=None):
ibis/backends/base/client.py:    def list_tables(self, like: str = None) -> list:
ibis/backends/pandas/client.py:    def list_tables(self, like=None):
@datapythonista datapythonista added refactor Issues or PRs related to refactoring the codebase backends Issues related to all backends labels Jul 24, 2021
@cpcloud cpcloud changed the title CLN: Standardize list_tables signature refactor: standardize list_tables signature Dec 28, 2021
@saulpw
Copy link
Contributor

saulpw commented Jan 13, 2022

I think it's a good idea to deprecate Database.list_tables and keep only Client.list_tables. It seems like backends want the ability to expose their own filtering mechanisms, and like may be either unsupported or insufficient; so we should add an abstract method which takes **kwargs instead of any guaranteed arguments. Then we can leave the existing Client.list_tables() as they are.

@cpcloud cpcloud added this to the 4.0.0 milestone Apr 19, 2022
@cpcloud cpcloud removed this from the 4.0.0 milestone Nov 7, 2022
gforsyth added a commit to gforsyth/ibis that referenced this issue Nov 7, 2022
Fixes ibis-project#2877 -- most of the differing signatures were removed in earlier
refactors -- this is the only one left (except for a mock fixture in the
test suite)
jcrist pushed a commit that referenced this issue Nov 8, 2022
Fixes #2877 -- most of the differing signatures were removed in earlier
refactors -- this is the only one left (except for a mock fixture in the
test suite)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backends Issues related to all backends refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants