From d4e9e9b99f64e0ced1d6eca2dbe8b296c21cd08f Mon Sep 17 00:00:00 2001 From: Gil Forsyth Date: Tue, 14 Mar 2023 10:04:19 -0400 Subject: [PATCH] refactor(ddl): unify exception messages and types --- ibis/backends/base/__init__.py | 4 +++- ibis/backends/base/sql/alchemy/__init__.py | 27 +++++++++++----------- ibis/backends/bigquery/__init__.py | 14 ++++++----- ibis/backends/clickhouse/__init__.py | 7 +++++- ibis/backends/impala/__init__.py | 13 ++++++----- ibis/backends/pandas/__init__.py | 20 ++++++++-------- ibis/backends/pyspark/__init__.py | 22 ++++++++++-------- 7 files changed, 61 insertions(+), 46 deletions(-) diff --git a/ibis/backends/base/__init__.py b/ibis/backends/base/__init__.py index dc06ce19cb974..20b50d7a6ffc9 100644 --- a/ibis/backends/base/__init__.py +++ b/ibis/backends/base/__init__.py @@ -811,6 +811,7 @@ def create_table( def drop_table( self, name: str, + *, database: str | None = None, force: bool = False, ) -> None: @@ -834,6 +835,7 @@ def create_view( self, name: str, obj: ir.Table, + *, database: str | None = None, overwrite: bool = False, ) -> ir.Table: @@ -859,7 +861,7 @@ def create_view( @abc.abstractmethod def drop_view( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: """Drop a view. diff --git a/ibis/backends/base/sql/alchemy/__init__.py b/ibis/backends/base/sql/alchemy/__init__.py index 68f1c954b06b9..b4590774bab33 100644 --- a/ibis/backends/base/sql/alchemy/__init__.py +++ b/ibis/backends/base/sql/alchemy/__init__.py @@ -11,7 +11,7 @@ import sqlalchemy as sa import ibis -import ibis.common.exceptions as exc +import ibis.common.exceptions as com import ibis.expr.operations as ops import ibis.expr.schema as sch import ibis.expr.types as ir @@ -214,6 +214,9 @@ def create_table( Table The table that was created. """ + if obj is None and schema is None: + raise com.IbisError("The schema or obj parameter is required") + import pandas as pd if isinstance(obj, pd.DataFrame): @@ -224,16 +227,13 @@ def create_table( database = None if database is not None: - raise NotImplementedError( - 'Creating tables from a different database is not yet implemented' + raise com.IbisInputError( + "Creating tables from a different database is not yet implemented" ) - if obj is None and schema is None: - raise ValueError('You must pass either an expression or a schema') - if obj is not None and schema is not None: if not obj.schema().equals(ibis.schema(schema)): - raise TypeError( + raise com.IbisTypeError( 'Expression schema is not equal to passed schema. ' 'Try passing the expression without the schema' ) @@ -305,7 +305,7 @@ def _table_from_schema( return sa.Table(name, sa.MetaData(), *columns, prefixes=prefixes) def drop_table( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: """Drop a table. @@ -323,8 +323,8 @@ def drop_table( database = None if database is not None: - raise NotImplementedError( - 'Dropping tables from a different database is not yet implemented' + raise com.IbisInputError( + "Dropping tables from a different database is not yet implemented" ) t = self._get_sqla_table(name, schema=database, autoload=False) @@ -525,7 +525,7 @@ def table( """ if database is not None: if not isinstance(database, str): - raise exc.IbisTypeError( + raise com.IbisTypeError( f"`database` must be a string; got {type(database)}" ) if database != self.current_database: @@ -537,7 +537,7 @@ def _insert_dataframe( self, table_name: str, df: pd.DataFrame, overwrite: bool ) -> None: if not self.inspector.has_table(table_name): - raise exc.IbisError(f"Cannot insert into non-existent table {table_name}") + raise com.IbisError(f"Cannot insert into non-existent table {table_name}") df.to_sql( table_name, self.con, @@ -698,6 +698,7 @@ def create_view( self, name: str, obj: ir.Table, + *, database: str | None = None, overwrite: bool = False, ) -> ir.Table: @@ -710,7 +711,7 @@ def create_view( return self.table(name, database=database) def drop_view( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: view = _drop_view(sa.table(name, schema=database), if_exists=not force) diff --git a/ibis/backends/bigquery/__init__.py b/ibis/backends/bigquery/__init__.py index 2ae86665064be..3df1c6d5d1ec4 100644 --- a/ibis/backends/bigquery/__init__.py +++ b/ibis/backends/bigquery/__init__.py @@ -15,6 +15,7 @@ from pydata_google_auth import cache import ibis +import ibis.common.exceptions as com import ibis.expr.operations as ops import ibis.expr.schema as sch import ibis.expr.types as ir @@ -447,16 +448,16 @@ def create_table( temp: bool | None = None, overwrite: bool = False, ) -> ir.Table: + if obj is None and schema is None: + raise com.IbisError("The schema or obj parameter is required") if temp is True: - raise NotImplementedError( + raise com.IbisInputError( "BigQuery backend does not yet support temporary tables" ) if overwrite is not False: - raise NotImplementedError( + raise com.IbisInputError( "BigQuery backend does not yet support overwriting tables" ) - if obj is None and schema is None: - raise ValueError("The schema or obj parameter is required") if schema is not None: table_id = self._fully_qualified_name(name, database) bigquery_schema = ibis_schema_to_bigquery_schema(schema) @@ -474,7 +475,7 @@ def create_table( return self.table(name, database=database) def drop_table( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: table_id = self._fully_qualified_name(name, database) self.client.delete_table(table_id, not_found_ok=not force) @@ -483,6 +484,7 @@ def create_view( self, name: str, obj: ir.Table, + *, database: str | None = None, overwrite: bool = False, ) -> ir.Table: @@ -494,7 +496,7 @@ def create_view( return self.table(name, database=database) def drop_view( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: self.drop_table(name=name, database=database, force=force) diff --git a/ibis/backends/clickhouse/__init__.py b/ibis/backends/clickhouse/__init__.py index de7c1f5fe36f6..e3fa74243c320 100644 --- a/ibis/backends/clickhouse/__init__.py +++ b/ibis/backends/clickhouse/__init__.py @@ -11,6 +11,7 @@ import toolz import ibis +import ibis.common.exceptions as com import ibis.config import ibis.expr.analysis as an import ibis.expr.operations as ops @@ -524,6 +525,9 @@ def create_table( replace = "OR REPLACE " * overwrite code = f"CREATE {replace}{tmp}TABLE {name}" + if obj is None and schema is None: + raise com.IbisError("The schema or obj parameter is required") + if schema is not None: code += f" ({schema})" @@ -557,6 +561,7 @@ def create_view( self, name: str, obj: ir.Table, + *, database: str | None = None, overwrite: bool = False, ) -> ir.Table: @@ -568,7 +573,7 @@ def create_view( return self.table(name, database=database) def drop_view( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: name = ".".join(filter(None, (database, name))) if_not_exists = "IF EXISTS " * force diff --git a/ibis/backends/impala/__init__.py b/ibis/backends/impala/__init__.py index b63e6bc47019c..868b58562a69b 100644 --- a/ibis/backends/impala/__init__.py +++ b/ibis/backends/impala/__init__.py @@ -583,6 +583,9 @@ def create_table( like_parquet Can specify instead of a schema """ + if obj is None and schema is None: + raise com.IbisError("The schema or obj parameter is required") + if temp is not None: raise NotImplementedError( "Impala backend does not yet support temporary tables" @@ -608,7 +611,7 @@ def create_table( path=location, ) ) - elif schema is not None: + else: # schema is not None if overwrite: self.drop_table(name, force=True) self.raw_sql( @@ -622,8 +625,6 @@ def create_table( partition=partition, ) ) - else: - raise com.IbisError('Must pass obj or schema') return self.table(name, database=database) def avro_file( @@ -916,7 +917,7 @@ def load_data( return table.load_data(path, overwrite=overwrite, partition=partition) def drop_table( - self, name: str, database: str | None = None, force: bool = False + self, name: str, *, database: str | None = None, force: bool = False ) -> None: """Drop an Impala table. @@ -951,7 +952,7 @@ def truncate_table(self, name: str, database: str | None = None) -> None: statement = TruncateTable(name, database=database) self.raw_sql(statement) - def drop_table_or_view(self, name, database=None, force=False): + def drop_table_or_view(self, name, *, database=None, force=False): """Drop view or table.""" try: self.drop_table(name, database=database) @@ -961,7 +962,7 @@ def drop_table_or_view(self, name, database=None, force=False): except Exception: # noqa: BLE001 raise e - def cache_table(self, table_name, database=None, pool='default'): + def cache_table(self, table_name, *, database=None, pool='default'): """Caches a table in cluster memory in the given pool. Parameters diff --git a/ibis/backends/pandas/__init__.py b/ibis/backends/pandas/__init__.py index fee56bbff82dc..9b3e7d49c7bc7 100644 --- a/ibis/backends/pandas/__init__.py +++ b/ibis/backends/pandas/__init__.py @@ -1,7 +1,6 @@ from __future__ import annotations import importlib -import warnings from functools import lru_cache from typing import TYPE_CHECKING, Any, Mapping, MutableMapping @@ -136,19 +135,19 @@ def create_table( ) -> ir.Table: """Create a table.""" if temp: - warnings.warn( + raise com.IbisInputError( "Passing `temp=True` to the Pandas backend create_table method has no " "effect: all tables are in memory and temporary. Avoid passing the " - "`temp` argument to silence this warning." + "`temp` argument." ) if database: - warnings.warn( + raise com.IbisInputError( "Passing `database` to the Pandas backend create_table method has no " "effect: Pandas cannot set a database. Avoid passing the `database` " - "argument to silence this warning." + "argument." ) if obj is None and schema is None: - raise com.IbisError('Must pass expr or schema') + raise com.IbisError("The schema or obj parameter is required") if obj is not None: if not self._supports_conversion(obj): @@ -175,21 +174,22 @@ def create_view( self, name: str, obj: ir.Table, + *, temp: bool | None = None, overwrite: bool = False, ) -> ir.Table: if temp: - warnings.warn( + raise com.IbisInputError( "Passing `temp=True` to the Pandas backend create_view method has no " "effect: all tables are in memory and temporary. Avoid passing the " - "`temp` argument to silence this warning" + "`temp` argument." ) return self.create_table(name, obj=obj, temp=None, overwrite=overwrite) - def drop_view(self, name: str, force: bool = False) -> None: + def drop_view(self, name: str, *, force: bool = False) -> None: self.drop_table(name, force=force) - def drop_table(self, name: str, force: bool = False) -> None: + def drop_table(self, name: str, *, force: bool = False) -> None: if not force and name in self.dictionary: raise com.IbisError( "Cannot drop existing table. Call drop_table with force=True to drop existing table." diff --git a/ibis/backends/pyspark/__init__.py b/ibis/backends/pyspark/__init__.py index 4cb304f204091..170c176e0b1b2 100644 --- a/ibis/backends/pyspark/__init__.py +++ b/ibis/backends/pyspark/__init__.py @@ -372,8 +372,10 @@ def create_table( """ import pandas as pd + if obj is None and schema is None: + raise com.IbisError("The schema or obj parameter is required") if temp is True: - raise NotImplementedError( + raise com.IbisInputError( "PySpark backend does not yet support temporary tables" ) if obj is not None: @@ -395,7 +397,7 @@ def create_table( can_exist=overwrite, format=format, ) - elif schema is not None: + else: statement = ddl.CreateTableWithSchema( name, schema, @@ -403,8 +405,6 @@ def create_table( format=format, can_exist=overwrite, ) - else: - raise com.IbisError('Must pass expr or schema') self.raw_sql(statement.compile()) return self.table(name, database=database) @@ -415,7 +415,8 @@ def _register_in_memory_table(self, op: ops.InMemoryTable) -> None: def create_view( self, name: str, - expr: ir.Table, + obj: ir.Table, + *, database: str | None = None, temp: bool | None = None, overwrite: bool = False, @@ -426,7 +427,7 @@ def create_view( ---------- name View name - expr + obj Expression to use for the view database Database name @@ -440,7 +441,7 @@ def create_view( Table The created view """ - ast = self.compiler.to_ast(expr) + ast = self.compiler.to_ast(obj) select = ast.queries[0] statement = ddl.CreateView( name, @@ -455,24 +456,27 @@ def create_view( def drop_table( self, name: str, + *, database: str | None = None, force: bool = False, ) -> None: """Drop a table.""" - self.drop_table_or_view(name, database, force) + self.drop_table_or_view(name, database=database, force=force) def drop_view( self, name: str, + *, database: str | None = None, force: bool = False, ): """Drop a view.""" - self.drop_table_or_view(name, database, force) + self.drop_table_or_view(name, database=database, force=force) def drop_table_or_view( self, name: str, + *, database: str | None = None, force: bool = False, ) -> None: