Skip to content

Commit

Permalink
Make edits according to review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nguyenv committed Apr 25, 2024
1 parent d704b21 commit e2a45f4
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
21 changes: 11 additions & 10 deletions apis/python/src/tiledbsoma/_arrow_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@
pa.date32(): TypeError("32-bit date - unsupported type (use TimestampType)"),
pa.date64(): TypeError("64-bit date - unsupported type (use TimestampType)"),
}
"""Dict of types unsupported by to_pandas_dtype, which require overrides for
use in TileDB Attributes (aka DataFrame non-indexe columns).
If the value is an instance of Exception, it will be raised.
IMPORTANT: ALL non-primitive types supported by TileDB must be in this table.
"""

_PYARROW_TO_CARROW: Dict[pa.DataType, str] = {
pa.bool_(): "b",
Expand All @@ -68,13 +75,6 @@
pa.timestamp("us"): "tsu:",
pa.timestamp("ns"): "tsn:",
}
"""Dict of types unsupported by to_pandas_dtype, which require overrides for
use in TileDB Attributes (aka DataFrame non-indexe columns).
If the value is an instance of Exception, it will be raised.
IMPORTANT: ALL non-primitive types supported by TileDB must be in this table.
"""

# Same as _ARROW_TO_TDB_ATTR, but used for DataFrame indexed columns, aka TileDB Dimensions.
# Any type system differences from the base-case Attr should be added here.
Expand Down Expand Up @@ -258,6 +258,7 @@ def df_to_arrow(df: pd.DataFrame) -> pa.Table:


def pyarrow_to_carrow_type(pa_type: pa.DataType) -> str:
if pa_type not in _PYARROW_TO_CARROW:
raise TypeError(f"Invalid pyarrow type {pa_type}")
return _PYARROW_TO_CARROW[pa_type]
try:
return _PYARROW_TO_CARROW[pa_type]
except KeyError:
raise TypeError(f"Invalid pyarrow type {pa_type}") from None
4 changes: 2 additions & 2 deletions apis/python/src/tiledbsoma/_dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def create(
index_column_data, schema=pa.schema(index_column_schema)
)

plt_cfg = _util.set_clib_platform_config(platform_config)
plt_cfg = _util.build_clib_platform_config(platform_config)
timestamp_ms = context._open_timestamp_ms(tiledb_timestamp)
try:
clib.SOMADataFrame.create(
Expand All @@ -262,7 +262,7 @@ def create(
timestamp=(0, timestamp_ms),
)
except SOMAError as e:
map_exception_for_create(e, uri)
raise map_exception_for_create(e, uri) from None

handle = cls._wrapper_type.open(uri, "w", context, tiledb_timestamp)
return cls(
Expand Down
14 changes: 7 additions & 7 deletions apis/python/src/tiledbsoma/_exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,19 @@ def is_duplicate_group_key_error(e: Union[SOMAError, tiledb.TileDBError]) -> boo


def is_domain_setting_error(e: SOMAError) -> bool:
"""Given a SOMAError, return whether it attempted to create the ArraySchema
but resulted in a
"""Given a SOMAError, return whether it attempted to create
the ArraySchema but the passed in domain was invalid.
Lifecycle: maturing
"""
return "Cannot set domain" in str(e)


def map_exception_for_create(e: SOMAError, uri: str) -> None:
def map_exception_for_create(e: SOMAError, uri: str) -> Exception:
if is_already_exists_error(e):
raise AlreadyExistsError(f"{uri!r} already exists")
return AlreadyExistsError(f"{uri!r} already exists")
if is_not_createable_error(e):
raise NotCreateableError(f"{uri!r} cannot be created")
return NotCreateableError(f"{uri!r} cannot be created")

Check warning on line 157 in apis/python/src/tiledbsoma/_exception.py

View check run for this annotation

Codecov / codecov/patch

apis/python/src/tiledbsoma/_exception.py#L157

Added line #L157 was not covered by tests
if is_domain_setting_error(e):
raise ValueError(e)
raise
return ValueError(e)
return e

Check warning on line 160 in apis/python/src/tiledbsoma/_exception.py

View check run for this annotation

Codecov / codecov/patch

apis/python/src/tiledbsoma/_exception.py#L160

Added line #L160 was not covered by tests
4 changes: 2 additions & 2 deletions apis/python/src/tiledbsoma/_sparse_nd_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def create(
)

carrow_type = pyarrow_to_carrow_type(type)
plt_cfg = _util.set_clib_platform_config(platform_config)
plt_cfg = _util.build_clib_platform_config(platform_config)
timestamp_ms = context._open_timestamp_ms(tiledb_timestamp)
try:
clib.SOMASparseNDArray.create(
Expand All @@ -149,7 +149,7 @@ def create(
timestamp=(0, timestamp_ms),
)
except SOMAError as e:
map_exception_for_create(e, uri)
raise map_exception_for_create(e, uri) from None

handle = cls._wrapper_type.open(uri, "w", context, tiledb_timestamp)
return cls(
Expand Down
4 changes: 2 additions & 2 deletions apis/python/src/tiledbsoma/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ def cast_values_to_target_schema(
2. Perform special casting for Boolean as it is represented in TileDB as
int8 but bits in Arrow
3. If there are new enumeration values in the values that are not yet
present in the schema, exten the enumeration to include these new values
present in the schema, extend the enumeration to include these new values
"""
target_schema = []
for i, input_field in enumerate(values.schema):
Expand Down Expand Up @@ -381,7 +381,7 @@ def cast_values_to_target_schema(
return values.cast(pa.schema(target_schema, values.schema.metadata))


def set_clib_platform_config(
def build_clib_platform_config(
platform_config: Optional[options.PlatformConfig],
) -> Optional[clib.PlatformConfig]:
"""
Expand Down

0 comments on commit e2a45f4

Please sign in to comment.