diff --git a/.github/workflows/python-ci-single.yml b/.github/workflows/python-ci-single.yml index 2def6608a2..a423dcb5f9 100644 --- a/.github/workflows/python-ci-single.yml +++ b/.github/workflows/python-ci-single.yml @@ -128,14 +128,7 @@ jobs: env: TILEDB_SOMA_INIT_BUFFER_BYTES: 33554432 # accommodate tiny runners - - name: Run pytests for Python without new shape - shell: bash - # Setting PYTHONPATH ensures the tests load the in-tree source code under apis/python/src - # instead of the copy we `pip install`ed to site-packages above. That's needed for the code - # coverage analysis to work. - run: export SOMA_PY_NEW_SHAPE=false; PYTHONPATH=$(pwd)/apis/python/src python -m pytest --cov=apis/python/src --cov-report=xml apis/python/tests -v --durations=20 --maxfail=50 - - - name: Run pytests for Python with new shape + - name: Run pytests for Python shell: bash # Setting PYTHONPATH ensures the tests load the in-tree source code under apis/python/src # instead of the copy we `pip install`ed to site-packages above. That's needed for the code diff --git a/.github/workflows/r-ci.yml b/.github/workflows/r-ci.yml index b5e71f61b8..7c75fa2507 100644 --- a/.github/workflows/r-ci.yml +++ b/.github/workflows/r-ci.yml @@ -148,12 +148,7 @@ jobs: # -e "devtools::install(upgrade = FALSE)" \ # -e "testthat::test_local('tests/testthat', load_package = 'installed')" - - name: Test without new shape - if: ${{ matrix.covr == 'no' }} - run: export SOMA_R_NEW_SHAPE=false && cd apis/r/tests && Rscript testthat.R - - # https://github.com/single-cell-data/TileDB-SOMA/issues/2407 - - name: Test with new shape + - name: Test if: ${{ matrix.covr == 'no' }} run: cd apis/r/tests && Rscript testthat.R diff --git a/.github/workflows/r-python-interop-testing.yml b/.github/workflows/r-python-interop-testing.yml index 8f1f9ae703..994c0e1971 100644 --- a/.github/workflows/r-python-interop-testing.yml +++ b/.github/workflows/r-python-interop-testing.yml @@ -110,14 +110,7 @@ jobs: - name: Update Packages run: Rscript -e 'update.packages(ask=FALSE)' - - name: Interop tests with new-shape feature flag off + - name: Interop tests run: python -m pytest apis/system/tests/ env: TILEDB_SOMA_INIT_BUFFER_BYTES: 33554432 # accommodate tiny runners - - - name: Interop tests with new-shape feature flag on - run: python -m pytest apis/system/tests/ - env: - TILEDB_SOMA_INIT_BUFFER_BYTES: 33554432 # accommodate tiny runners - SOMA_PY_NEW_SHAPE: true - SOMA_R_NEW_SHAPE: true diff --git a/apis/python/src/tiledbsoma/_dataframe.py b/apis/python/src/tiledbsoma/_dataframe.py index 5a58ec22b9..a7fb3aba9a 100644 --- a/apis/python/src/tiledbsoma/_dataframe.py +++ b/apis/python/src/tiledbsoma/_dataframe.py @@ -24,8 +24,6 @@ from somacore import options from typing_extensions import Self -from tiledbsoma._flags import NEW_SHAPE_FEATURE_FLAG_ENABLED - from . import _arrow_types, _util from . import pytiledbsoma as clib from ._constants import SOMA_JOINID @@ -308,16 +306,12 @@ def create( # [4] core current domain hi index_column_schema.append(pa_field) - if NEW_SHAPE_FEATURE_FLAG_ENABLED: - - index_column_data[pa_field.name] = [ - *slot_core_max_domain, - extent, - *slot_core_current_domain, - ] - else: - index_column_data[pa_field.name] = [*slot_core_current_domain, extent] + index_column_data[pa_field.name] = [ + *slot_core_max_domain, + extent, + *slot_core_current_domain, + ] index_column_info = pa.RecordBatch.from_pydict( index_column_data, schema=pa.schema(index_column_schema) @@ -916,7 +910,7 @@ def _fill_out_slot_soma_domain( # will (and must) ignore these when creating the TileDB schema. slot_domain = "", "" elif np.issubdtype(dtype, NPInteger): - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: # Core max domain is immutable. If unspecified, it should be as big # as possible since it can never be resized. iinfo = np.iinfo(cast(NPInteger, dtype)) @@ -936,7 +930,7 @@ def _fill_out_slot_soma_domain( # not 0. slot_domain = 0, 0 elif np.issubdtype(dtype, NPFloating): - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: finfo = np.finfo(cast(NPFloating, dtype)) slot_domain = finfo.min, finfo.max saturated_range = True @@ -957,7 +951,7 @@ def _fill_out_slot_soma_domain( # value representable by domain type. Reduce domain max by 1 tile extent # to allow for expansion. elif dtype == "datetime64[s]": - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: iinfo = np.iinfo(cast(NPInteger, np.int64)) slot_domain = np.datetime64(iinfo.min + 1, "s"), np.datetime64( iinfo.max - 1000000, "s" @@ -965,7 +959,7 @@ def _fill_out_slot_soma_domain( else: slot_domain = np.datetime64(0, "s"), np.datetime64(0, "s") elif dtype == "datetime64[ms]": - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: iinfo = np.iinfo(cast(NPInteger, np.int64)) slot_domain = np.datetime64(iinfo.min + 1, "ms"), np.datetime64( iinfo.max - 1000000, "ms" @@ -973,7 +967,7 @@ def _fill_out_slot_soma_domain( else: slot_domain = np.datetime64(0, "ms"), np.datetime64(0, "ms") elif dtype == "datetime64[us]": - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: iinfo = np.iinfo(cast(NPInteger, np.int64)) slot_domain = np.datetime64(iinfo.min + 1, "us"), np.datetime64( iinfo.max - 1000000, "us" @@ -981,7 +975,7 @@ def _fill_out_slot_soma_domain( else: slot_domain = np.datetime64(0, "us"), np.datetime64(0, "us") elif dtype == "datetime64[ns]": - if is_max_domain or not NEW_SHAPE_FEATURE_FLAG_ENABLED: + if is_max_domain: iinfo = np.iinfo(cast(NPInteger, np.int64)) slot_domain = np.datetime64(iinfo.min + 1, "ns"), np.datetime64( iinfo.max - 1000000, "ns" diff --git a/apis/python/src/tiledbsoma/_dense_nd_array.py b/apis/python/src/tiledbsoma/_dense_nd_array.py index a22a4d8450..a64c83745c 100644 --- a/apis/python/src/tiledbsoma/_dense_nd_array.py +++ b/apis/python/src/tiledbsoma/_dense_nd_array.py @@ -20,7 +20,7 @@ from ._arrow_types import pyarrow_to_carrow_type from ._common_nd_array import NDArray from ._exception import SOMAError, map_exception_for_create -from ._flags import DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN, NEW_SHAPE_FEATURE_FLAG_ENABLED +from ._flags import DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN from ._tdb_handles import DenseNDArrayWrapper from ._types import OpenTimestamp, Slice, StatusAndReason from ._util import dense_indices_to_shape @@ -122,7 +122,7 @@ def create( if dim_shape is None: raise ValueError("DenseNDArray shape slots must be numeric") - if NEW_SHAPE_FEATURE_FLAG_ENABLED and DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: + if DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: dim_capacity, dim_extent = cls._dim_capacity_and_extent( dim_name, # The user specifies current domain -- this is the max domain @@ -368,7 +368,7 @@ def tiledbsoma_upgrade_shape( 1.15 release notes. Raises an error if the new shape exceeds maxshape in any dimension. Raises an error if the array already has a shape. """ - if NEW_SHAPE_FEATURE_FLAG_ENABLED and DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: + if DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: if check_only: return self._handle.tiledbsoma_can_upgrade_shape(newshape) else: diff --git a/apis/python/src/tiledbsoma/_flags.py b/apis/python/src/tiledbsoma/_flags.py index 53a4bd7588..84d05cfcbe 100644 --- a/apis/python/src/tiledbsoma/_flags.py +++ b/apis/python/src/tiledbsoma/_flags.py @@ -12,8 +12,6 @@ # removed once https://github.com/single-cell-data/TileDB-SOMA/issues/2407 is # complete. -NEW_SHAPE_FEATURE_FLAG_ENABLED = os.getenv("SOMA_PY_NEW_SHAPE") != "false" - DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN = clib.embedded_version_triple() >= (2, 27, 0) # Temporary for # https://github.com/single-cell-data/TileDB-SOMA/issues/2407: diff --git a/apis/python/src/tiledbsoma/_point_cloud_dataframe.py b/apis/python/src/tiledbsoma/_point_cloud_dataframe.py index ef5edc40bb..5c9f9d3b28 100644 --- a/apis/python/src/tiledbsoma/_point_cloud_dataframe.py +++ b/apis/python/src/tiledbsoma/_point_cloud_dataframe.py @@ -29,7 +29,6 @@ _revise_domain_for_extent, ) from ._exception import SOMAError, map_exception_for_create -from ._flags import NEW_SHAPE_FEATURE_FLAG_ENABLED from ._query_condition import QueryCondition from ._read_iters import TableReadIter from ._spatial_dataframe import SpatialDataFrame @@ -215,16 +214,12 @@ def create( # [4] core current domain hi index_column_schema.append(pa_field) - if NEW_SHAPE_FEATURE_FLAG_ENABLED: - index_column_data[pa_field.name] = [ - *slot_core_max_domain, - extent, - *slot_core_current_domain, - ] - - else: - index_column_data[pa_field.name] = [*slot_core_current_domain, extent] + index_column_data[pa_field.name] = [ + *slot_core_max_domain, + extent, + *slot_core_current_domain, + ] index_column_info = pa.RecordBatch.from_pydict( index_column_data, schema=pa.schema(index_column_schema) diff --git a/apis/python/src/tiledbsoma/_sparse_nd_array.py b/apis/python/src/tiledbsoma/_sparse_nd_array.py index 9434754770..55ab1678f1 100644 --- a/apis/python/src/tiledbsoma/_sparse_nd_array.py +++ b/apis/python/src/tiledbsoma/_sparse_nd_array.py @@ -26,8 +26,6 @@ from somacore.options import PlatformConfig from typing_extensions import Self -from tiledbsoma._flags import NEW_SHAPE_FEATURE_FLAG_ENABLED - from . import _util # This package's pybind11 code @@ -162,41 +160,31 @@ def create( # [3] core current domain lo # [4] core current domain hi - if NEW_SHAPE_FEATURE_FLAG_ENABLED: - dim_capacity, dim_extent = cls._dim_capacity_and_extent( - dim_name, - # The user specifies current domain -- this is the max domain - # which is taken from the max ranges for the dim datatype. - # We pass None here to detect those. - None, - len(shape), - TileDBCreateOptions.from_platform_config(platform_config), - ) - - if dim_shape == 0: - raise ValueError("SparseNDArray shape slots must be at least 1") - if dim_shape is None: - # Core current-domain semantics are (lo, hi) with both - # inclusive, with lo <= hi. This means smallest is (0, 0) - # which is shape 1, not 0. - dim_shape = 1 - - index_column_data[pa_field.name] = [ - 0, - dim_capacity - 1, - dim_extent, - 0, - dim_shape - 1, - ] + dim_capacity, dim_extent = cls._dim_capacity_and_extent( + dim_name, + # The user specifies current domain -- this is the max domain + # which is taken from the max ranges for the dim datatype. + # We pass None here to detect those. + None, + len(shape), + TileDBCreateOptions.from_platform_config(platform_config), + ) - else: - dim_capacity, dim_extent = cls._dim_capacity_and_extent( - dim_name, - dim_shape, - len(shape), - TileDBCreateOptions.from_platform_config(platform_config), - ) - index_column_data[pa_field.name] = [0, dim_capacity - 1, dim_extent] + if dim_shape == 0: + raise ValueError("SparseNDArray shape slots must be at least 1") + if dim_shape is None: + # Core current-domain semantics are (lo, hi) with both + # inclusive, with lo <= hi. This means smallest is (0, 0) + # which is shape 1, not 0. + dim_shape = 1 + + index_column_data[pa_field.name] = [ + 0, + dim_capacity - 1, + dim_extent, + 0, + dim_shape - 1, + ] index_column_info = pa.RecordBatch.from_pydict( index_column_data, schema=pa.schema(index_column_schema) diff --git a/apis/python/src/tiledbsoma/io/ingest.py b/apis/python/src/tiledbsoma/io/ingest.py index 465e7d058f..0b2b0bf8ae 100644 --- a/apis/python/src/tiledbsoma/io/ingest.py +++ b/apis/python/src/tiledbsoma/io/ingest.py @@ -64,10 +64,6 @@ NotCreateableError, SOMAError, ) -from .._flags import ( - DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN, - NEW_SHAPE_FEATURE_FLAG_ENABLED, -) from .._soma_array import SOMAArray from .._soma_object import AnySOMAObject, SOMAObject from .._tdb_handles import RawHandle @@ -1211,9 +1207,7 @@ def _write_dataframe_impl( try: # Note: tiledbsoma.io creates dataframes with soma_joinid being the one # and only index column. - domain = None - if NEW_SHAPE_FEATURE_FLAG_ENABLED: - domain = ((0, shape - 1),) + domain = ((0, shape - 1),) soma_df = DataFrame.create( df_uri, schema=arrow_table.schema, @@ -1318,17 +1312,14 @@ def _create_from_matrix( try: shape: Sequence[Union[int, None]] = () # A SparseNDArray must be appendable in soma.io. - if NEW_SHAPE_FEATURE_FLAG_ENABLED: - # Instead of - # shape = tuple(int(e) for e in matrix.shape) - # we consult the registration mapping. This is important - # in the case when multiple H5ADs/AnnDatas are being - # ingested to an experiment which doesn't pre-exist. - shape = (axis_0_mapping.get_shape(), axis_1_mapping.get_shape()) - elif cls.is_sparse or DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: - shape = tuple(None for _ in matrix.shape) - else: - shape = matrix.shape + + # Instead of + # shape = tuple(int(e) for e in matrix.shape) + # we consult the registration mapping. This is important + # in the case when multiple H5ADs/AnnDatas are being + # ingested to an experiment which doesn't pre-exist. + shape = (axis_0_mapping.get_shape(), axis_1_mapping.get_shape()) + soma_ndarray = cls.create( uri, type=pa.from_numpy_dtype(matrix.dtype), diff --git a/apis/python/tests/test_basic_anndata_io.py b/apis/python/tests/test_basic_anndata_io.py index 0b6d8f95b9..1f560a0d61 100644 --- a/apis/python/tests/test_basic_anndata_io.py +++ b/apis/python/tests/test_basic_anndata_io.py @@ -1336,10 +1336,9 @@ def test_nan_append(conftest_pbmc_small, dtype, nans, new_obs_ids): var_field_name="var_id", ) - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - nobs = rd.get_obs_shape() - nvars = rd.get_var_shapes() - tiledbsoma.io.resize_experiment(SOMA_URI, nobs=nobs, nvars=nvars) + nobs = rd.get_obs_shape() + nvars = rd.get_var_shapes() + tiledbsoma.io.resize_experiment(SOMA_URI, nobs=nobs, nvars=nvars) # Append the second anndata object tiledbsoma.io.from_anndata( diff --git a/apis/python/tests/test_dataframe.py b/apis/python/tests/test_dataframe.py index b24e4f90e0..fe4f445afe 100644 --- a/apis/python/tests/test_dataframe.py +++ b/apis/python/tests/test_dataframe.py @@ -82,8 +82,7 @@ def test_dataframe(tmp_path, arrow_schema): pydict["quux"] = [True, False, False, True, False] rb = pa.Table.from_pydict(pydict) - if soma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - sdf.tiledbsoma_resize_soma_joinid_shape(len(rb)) + sdf.tiledbsoma_resize_soma_joinid_shape(len(rb)) sdf.write(rb) @@ -95,11 +94,7 @@ def test_dataframe(tmp_path, arrow_schema): assert sdf.count == 5 assert len(sdf) == 5 - # More to come on https://github.com/single-cell-data/TileDB-SOMA/issues/2407 - assert ( - sdf.tiledbsoma_has_upgraded_domain - == soma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED - ) + assert sdf.tiledbsoma_has_upgraded_domain with pytest.raises(AttributeError): assert sdf.shape is None diff --git a/apis/python/tests/test_registration_mappings.py b/apis/python/tests/test_registration_mappings.py index 44ac60778b..3247f4bf19 100644 --- a/apis/python/tests/test_registration_mappings.py +++ b/apis/python/tests/test_registration_mappings.py @@ -382,10 +382,9 @@ def test_multiples_without_experiment( var_field_name=var_field_name, ) - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - nobs = rd.get_obs_shape() - nvars = rd.get_var_shapes() - tiledbsoma.io.resize_experiment(experiment_uri, nobs=nobs, nvars=nvars) + nobs = rd.get_obs_shape() + nvars = rd.get_var_shapes() + tiledbsoma.io.resize_experiment(experiment_uri, nobs=nobs, nvars=nvars) else: # "Append" all the H5ADs where no experiment exists yet. @@ -456,13 +455,13 @@ def test_multiples_without_experiment( h5ad_file_names[permutation[2]], h5ad_file_names[permutation[3]], ]: - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - if tiledbsoma.Experiment.exists(experiment_uri): - tiledbsoma.io.resize_experiment( - experiment_uri, - nobs=rd.get_obs_shape(), - nvars=rd.get_var_shapes(), - ) + + if tiledbsoma.Experiment.exists(experiment_uri): + tiledbsoma.io.resize_experiment( + experiment_uri, + nobs=rd.get_obs_shape(), + nvars=rd.get_var_shapes(), + ) tiledbsoma.io.from_h5ad( experiment_uri, @@ -726,12 +725,11 @@ def test_append_items_with_experiment(obs_field_name, var_field_name): original = adata2.copy() - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - tiledbsoma.io.resize_experiment( - soma1, - nobs=rd.get_obs_shape(), - nvars=rd.get_var_shapes(), - ) + tiledbsoma.io.resize_experiment( + soma1, + nobs=rd.get_obs_shape(), + nvars=rd.get_var_shapes(), + ) with tiledbsoma.Experiment.open(soma1, "w") as exp1: tiledbsoma.io.append_obs( @@ -856,12 +854,11 @@ def test_append_with_disjoint_measurements( var_field_name=var_field_name, ) - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - tiledbsoma.io.resize_experiment( - soma_uri, - nobs=rd.get_obs_shape(), - nvars=rd.get_var_shapes(), - ) + tiledbsoma.io.resize_experiment( + soma_uri, + nobs=rd.get_obs_shape(), + nvars=rd.get_var_shapes(), + ) tiledbsoma.io.from_anndata( soma_uri, @@ -1218,12 +1215,11 @@ def test_enum_bit_width_append(tmp_path, all_at_once, nobs_a, nobs_b): soma_uri, adata, measurement_name=measurement_name, registration_mapping=rd ) - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - tiledbsoma.io.resize_experiment( - soma_uri, - nobs=rd.get_obs_shape(), - nvars=rd.get_var_shapes(), - ) + tiledbsoma.io.resize_experiment( + soma_uri, + nobs=rd.get_obs_shape(), + nvars=rd.get_var_shapes(), + ) tiledbsoma.io.from_anndata( soma_uri, bdata, measurement_name=measurement_name, registration_mapping=rd @@ -1243,12 +1239,11 @@ def test_enum_bit_width_append(tmp_path, all_at_once, nobs_a, nobs_b): assert rd.get_obs_shape() == nobs_a + nobs_b assert rd.get_var_shapes() == {"meas": 4} - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - tiledbsoma.io.resize_experiment( - soma_uri, - nobs=rd.get_obs_shape(), - nvars=rd.get_var_shapes(), - ) + tiledbsoma.io.resize_experiment( + soma_uri, + nobs=rd.get_obs_shape(), + nvars=rd.get_var_shapes(), + ) tiledbsoma.io.from_anndata( soma_uri, bdata, measurement_name=measurement_name, registration_mapping=rd diff --git a/apis/python/tests/test_shape.py b/apis/python/tests/test_shape.py index 13362da3f1..690127cfa8 100644 --- a/apis/python/tests/test_shape.py +++ b/apis/python/tests/test_shape.py @@ -51,11 +51,7 @@ def test_sparse_nd_array_basics( assert snda.shape == arg_shape - # More to come on https://github.com/single-cell-data/TileDB-SOMA/issues/2407 - assert ( - snda.tiledbsoma_has_upgraded_shape - == tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED - ) + assert snda.tiledbsoma_has_upgraded_shape # Before current-domain support: shape is maxshape. # @@ -64,11 +60,9 @@ def test_sparse_nd_array_basics( # involving R compatibility, and leaving room for a single tile # capacity, etc ... we could check for some magic value but it suffices # to check that it's over 2 billion.) - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - for e in snda.maxshape: - assert e > 2_000_000_000 - else: - assert snda.shape == snda.maxshape + + for e in snda.maxshape: + assert e > 2_000_000_000 # No data have been written for this test case assert snda.non_empty_domain() == tuple([(0, 0)] * ndim) @@ -95,11 +89,8 @@ def test_sparse_nd_array_basics( with tiledbsoma.SparseNDArray.open(uri) as snda: assert snda.shape == arg_shape # This will change with current-domain support - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - for e in snda.maxshape: - assert e > 2_000_000_000 - else: - assert snda.shape == snda.maxshape + for e in snda.maxshape: + assert e > 2_000_000_000 assert snda.non_empty_domain() == coords # Test reads out of bounds @@ -119,83 +110,75 @@ def test_sparse_nd_array_basics( with tiledbsoma.SparseNDArray.open(uri) as snda: assert snda.shape == arg_shape - if not tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - with tiledbsoma.SparseNDArray.open(uri) as snda: - ok, msg = snda.tiledbsoma_upgrade_shape(arg_shape, check_only=True) - assert ok - assert msg == "" - - else: - - with tiledbsoma.SparseNDArray.open(uri) as snda: - ok, msg = snda.tiledbsoma_upgrade_shape(arg_shape, check_only=True) - assert not ok - assert ( - msg - == "tiledbsoma_can_upgrade_shape: array already has a shape: please use resize" - ) - - # Test resize down - new_shape = tuple([arg_shape[i] - 50 for i in range(ndim)]) - with tiledbsoma.SparseNDArray.open(uri, "w") as snda: - (ok, msg) = snda.resize(new_shape, check_only=True) - assert not ok - assert msg == "can_resize for soma_dim_0: new 50 < existing shape 100" - # TODO: check draft spec - # with pytest.raises(ValueError): - with pytest.raises(tiledbsoma.SOMAError): - snda.resize(new_shape) - - with tiledbsoma.SparseNDArray.open(uri) as snda: - assert snda.shape == arg_shape - - # Test writes out of bounds - with tiledbsoma.SparseNDArray.open(uri, "w") as snda: - with pytest.raises(tiledbsoma.SOMAError): - dikt = {name: [shape + 20] for name, shape in zip(dim_names, arg_shape)} - dikt["soma_data"] = [30] - table = pa.Table.from_pydict(dikt) - snda.write(table) + with tiledbsoma.SparseNDArray.open(uri) as snda: + ok, msg = snda.tiledbsoma_upgrade_shape(arg_shape, check_only=True) + assert not ok + assert ( + msg + == "tiledbsoma_can_upgrade_shape: array already has a shape: please use resize" + ) - # Test resize - new_shape = tuple([arg_shape[i] + 50 for i in range(ndim)]) - with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + # Test resize down + new_shape = tuple([arg_shape[i] - 50 for i in range(ndim)]) + with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + (ok, msg) = snda.resize(new_shape, check_only=True) + assert not ok + assert msg == "can_resize for soma_dim_0: new 50 < existing shape 100" + # TODO: check draft spec + # with pytest.raises(ValueError): + with pytest.raises(tiledbsoma.SOMAError): snda.resize(new_shape) - dikt = {} - for i in range(ndim): - dikt[dim_names[i]] = [arg_shape[i] + 20] - dikt["soma_data"] = pa.array([34.5], type=element_dtype) + with tiledbsoma.SparseNDArray.open(uri) as snda: + assert snda.shape == arg_shape + + # Test writes out of bounds + with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + with pytest.raises(tiledbsoma.SOMAError): + dikt = {name: [shape + 20] for name, shape in zip(dim_names, arg_shape)} + dikt["soma_data"] = [30] table = pa.Table.from_pydict(dikt) + snda.write(table) - # Re-test writes out of old bounds, within new bounds - with tiledbsoma.SparseNDArray.open(uri, "w") as snda: - # Implicitly checking there's no raise - snda.write(table) + # Test resize + new_shape = tuple([arg_shape[i] + 50 for i in range(ndim)]) + with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + snda.resize(new_shape) - # Re-test reads out of old bounds, within new bounds - with tiledbsoma.SparseNDArray.open(uri) as snda: - assert snda.shape == new_shape + dikt = {} + for i in range(ndim): + dikt[dim_names[i]] = [arg_shape[i] + 20] + dikt["soma_data"] = pa.array([34.5], type=element_dtype) + table = pa.Table.from_pydict(dikt) - coords = tuple([(arg_shape[i] + 20,) for i in range(ndim)]) - # Implicitly checking there's no raise - readback = snda.read(coords).tables().concat() - assert readback == table + # Re-test writes out of old bounds, within new bounds + with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + # Implicitly checking there's no raise + snda.write(table) + # Re-test reads out of old bounds, within new bounds with tiledbsoma.SparseNDArray.open(uri) as snda: assert snda.shape == new_shape - (ok, msg) = snda.resize(new_shape, check_only=True) - assert ok - assert msg == "" + coords = tuple([(arg_shape[i] + 20,) for i in range(ndim)]) + # Implicitly checking there's no raise + readback = snda.read(coords).tables().concat() + assert readback == table - too_small = tuple(e - 1 for e in new_shape) - (ok, msg) = snda.resize(too_small, check_only=True) - assert not ok - assert msg == "can_resize for soma_dim_0: new 149 < existing shape 150" + with tiledbsoma.SparseNDArray.open(uri) as snda: + assert snda.shape == new_shape - with tiledbsoma.SparseNDArray.open(uri, "w") as snda: - (ok, msg) = snda.resize(new_shape, check_only=True) + (ok, msg) = snda.resize(new_shape, check_only=True) + assert ok + assert msg == "" + + too_small = tuple(e - 1 for e in new_shape) + (ok, msg) = snda.resize(too_small, check_only=True) + assert not ok + assert msg == "can_resize for soma_dim_0: new 149 < existing shape 150" + + with tiledbsoma.SparseNDArray.open(uri, "w") as snda: + (ok, msg) = snda.resize(new_shape, check_only=True) def test_dense_nd_array_basics(tmp_path): @@ -228,10 +211,7 @@ def test_dense_nd_array_basics(tmp_path): else: assert dnda.shape == (100, 200) - if ( - tiledbsoma._flags.DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN - and tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED - ): + if tiledbsoma._flags.DENSE_ARRAYS_CAN_HAVE_CURRENT_DOMAIN: with tiledbsoma.DenseNDArray.open(uri) as dnda: ok, msg = dnda.tiledbsoma_upgrade_shape((600, 700), check_only=True) assert not ok @@ -315,70 +295,59 @@ def test_dataframe_basics(tmp_path, soma_joinid_domain, index_column_names): has_sjid_dim = "soma_joinid" in index_column_names if has_sjid_dim: assert sdf._maybe_soma_joinid_shape == 1 + soma_joinid_domain[1] - if not tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - assert sdf._maybe_soma_joinid_maxshape == 1 + soma_joinid_domain[1] else: assert sdf._maybe_soma_joinid_shape is None - if not tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - assert sdf._maybe_soma_joinid_maxshape is None assert len(sdf.non_empty_domain()) == len(index_column_names) # This may be None if soma_joinid is not an index column shape_at_create = sdf._maybe_soma_joinid_shape - if tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - - # Test resize down - new_shape = 0 - with tiledbsoma.DataFrame.open(uri, "w") as sdf: - ok, msg = sdf.tiledbsoma_resize_soma_joinid_shape( - new_shape, check_only=True + # Test resize down + new_shape = 0 + with tiledbsoma.DataFrame.open(uri, "w") as sdf: + ok, msg = sdf.tiledbsoma_resize_soma_joinid_shape(new_shape, check_only=True) + if has_soma_joinid_dim: + # TODO: check draft spec + # with pytest.raises(ValueError): + assert not ok + assert ( + "tiledbsoma_resize_soma_joinid_shape: new soma_joinid shape 0 < existing shape" + in msg ) - if has_soma_joinid_dim: - # TODO: check draft spec - # with pytest.raises(ValueError): - assert not ok - assert ( - "tiledbsoma_resize_soma_joinid_shape: new soma_joinid shape 0 < existing shape" - in msg - ) - with pytest.raises(tiledbsoma.SOMAError): - sdf.tiledbsoma_resize_soma_joinid_shape(new_shape) - else: - assert ok - assert msg == "" + with pytest.raises(tiledbsoma.SOMAError): sdf.tiledbsoma_resize_soma_joinid_shape(new_shape) + else: + assert ok + assert msg == "" + sdf.tiledbsoma_resize_soma_joinid_shape(new_shape) - with tiledbsoma.DataFrame.open(uri) as sdf: - assert sdf._maybe_soma_joinid_shape == shape_at_create + with tiledbsoma.DataFrame.open(uri) as sdf: + assert sdf._maybe_soma_joinid_shape == shape_at_create - # Test writes out of bounds, before resize - offset = shape_at_create if has_soma_joinid_dim else 100 - data_dict["soma_joinid"] = [e + offset for e in data_dict["soma_joinid"]] - data = pa.Table.from_pydict(data_dict) + # Test writes out of bounds, before resize + offset = shape_at_create if has_soma_joinid_dim else 100 + data_dict["soma_joinid"] = [e + offset for e in data_dict["soma_joinid"]] + data = pa.Table.from_pydict(data_dict) - with tiledbsoma.DataFrame.open(uri, "w") as sdf: - if has_soma_joinid_dim: - with pytest.raises(tiledbsoma.SOMAError): - sdf.write(data) - else: + with tiledbsoma.DataFrame.open(uri, "w") as sdf: + if has_soma_joinid_dim: + with pytest.raises(tiledbsoma.SOMAError): sdf.write(data) + else: + sdf.write(data) - # Test resize - new_shape = 0 if shape_at_create is None else shape_at_create + 100 - with tiledbsoma.DataFrame.open(uri, "w") as sdf: - sdf.tiledbsoma_resize_soma_joinid_shape(new_shape) + # Test resize + new_shape = 0 if shape_at_create is None else shape_at_create + 100 + with tiledbsoma.DataFrame.open(uri, "w") as sdf: + sdf.tiledbsoma_resize_soma_joinid_shape(new_shape) - # Test writes out of old bounds, within new bounds, after resize - with tiledbsoma.DataFrame.open(uri, "w") as sdf: - sdf.write(data) + # Test writes out of old bounds, within new bounds, after resize + with tiledbsoma.DataFrame.open(uri, "w") as sdf: + sdf.write(data) def test_domain_mods(tmp_path): - if not tiledbsoma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - return - uri = tmp_path.as_posix() schema = pa.schema( diff --git a/apis/python/tests/test_sparse_nd_array.py b/apis/python/tests/test_sparse_nd_array.py index fc45c64126..403e806b6d 100644 --- a/apis/python/tests/test_sparse_nd_array.py +++ b/apis/python/tests/test_sparse_nd_array.py @@ -387,10 +387,7 @@ def test_sparse_nd_array_read_as_pandas( def test_sparse_nd_array_shaping(tmp_path, shape_is_nones, element_type): uri = tmp_path.as_posix() - if soma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - shape = [2, 3] - else: - shape = [None, None] if shape_is_nones else [2, 3] + shape = [2, 3] soma.SparseNDArray.create( uri, @@ -421,9 +418,8 @@ def test_sparse_nd_array_shaping(tmp_path, shape_is_nones, element_type): assert snda.nnz == 6 if shape_is_nones: - if soma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - with soma.SparseNDArray.open(uri, "w") as snda: - snda.resize([3, 3]) + with soma.SparseNDArray.open(uri, "w") as snda: + snda.resize([3, 3]) with soma.SparseNDArray.open(uri, "w") as snda: snda.write(batch2) else: @@ -1094,12 +1090,8 @@ def test_tile_extents(tmp_path): with soma.SparseNDArray.open(tmp_path.as_posix()) as A: dim_info = json.loads(A.config_options_from_schema().dims) - if soma._flags.NEW_SHAPE_FEATURE_FLAG_ENABLED: - assert int(dim_info["soma_dim_0"]["tile"]) == 2048 - assert int(dim_info["soma_dim_1"]["tile"]) == 2048 - else: - assert int(dim_info["soma_dim_0"]["tile"]) == 100 - assert int(dim_info["soma_dim_1"]["tile"]) == 2048 + assert int(dim_info["soma_dim_0"]["tile"]) == 2048 + assert int(dim_info["soma_dim_1"]["tile"]) == 2048 @pytest.mark.parametrize( diff --git a/apis/r/R/Init.R b/apis/r/R/Init.R index e26738f54a..45be659e05 100644 --- a/apis/r/R/Init.R +++ b/apis/r/R/Init.R @@ -29,13 +29,6 @@ } } -# This is temporary only. Please see: -# * https://github.com/single-cell-data/TileDB-SOMA/issues/2407 -# * https://github.com/single-cell-data/TileDB-SOMA/pull/2950 -.new_shape_feature_flag_is_enabled <- function() { - .pkgenv[["use_current_domain_transitional_internal_only"]] -} - # Temporary for # https://github.com/single-cell-data/TileDB-SOMA/issues/2407. # Once core 2.27 is released and we depend on it, this can go away. .dense_arrays_can_have_current_domain <- function() { @@ -63,13 +56,6 @@ } } -# This is temporary only. Please see: -# * https://github.com/single-cell-data/TileDB-SOMA/issues/2407 -# * https://github.com/single-cell-data/TileDB-SOMA/pull/2950 -.new_shape_feature_flag_is_enabled <- function() { - .pkgenv[["use_current_domain_transitional_internal_only"]] -} - #' Create and cache a SOMA Context Object #' #' @param config A named character vector with \sQuote{key} and \sQuote{value} pairs defining the diff --git a/apis/r/R/utils-arrow.R b/apis/r/R/utils-arrow.R index eff8613cff..b45981d7e2 100644 --- a/apis/r/R/utils-arrow.R +++ b/apis/r/R/utils-arrow.R @@ -494,25 +494,18 @@ get_domain_and_extent_dataframe <- function( requested_slot <- domain[[ind_col_name]] ind_cur_dom <- if (is.null(requested_slot)) { - if (.new_shape_feature_flag_is_enabled()) { - # New shape: if the slot is null, make the size as small - # as possible since current domain can only be resized upward. - # - # Core current-domain semantics are (lo, hi) with both - # inclusive, with lo <= hi. This means smallest is (0, 0) - # which is shape 1, not 0. - if (bit64::is.integer64(ind_max_dom)) { - c(bit64::as.integer64(0), bit64::as.integer64(0)) - } else if (is.integer(ind_max_dom)) { - c(0L, 0L) - } else { - c(0, 0) - } + # New shape: if the slot is null, make the size as small + # as possible since current domain can only be resized upward. + # + # Core current-domain semantics are (lo, hi) with both + # inclusive, with lo <= hi. This means smallest is (0, 0) + # which is shape 1, not 0. + if (bit64::is.integer64(ind_max_dom)) { + c(bit64::as.integer64(0), bit64::as.integer64(0)) + } else if (is.integer(ind_max_dom)) { + c(0L, 0L) } else { - # Old shape: if the slot is null, make the size as large - # as possible since there is not current domain, and the - # max domain is immutable. - ind_max_dom + c(0, 0) } } else { requested_slot @@ -522,49 +515,32 @@ get_domain_and_extent_dataframe <- function( if (ind_col_type_name %in% c("string", "large_utf8", "utf8")) ind_ext <- NA # https://github.com/single-cell-data/TileDB-SOMA/issues/2407 - if (.new_shape_feature_flag_is_enabled()) { - if (ind_col_type_name %in% c("string", "utf8", "large_utf8")) { - aa <- if (is.null(requested_slot)) { - arrow::arrow_array(c("", "", "", "", ""), ind_col_type) - } else { - arrow::arrow_array(c("", "", "", requested_slot[[1]], requested_slot[[2]]), ind_col_type) - } + if (ind_col_type_name %in% c("string", "utf8", "large_utf8")) { + aa <- if (is.null(requested_slot)) { + arrow::arrow_array(c("", "", "", "", ""), ind_col_type) } else { - # If they wanted (0, 99) then extent must be at most 100. - # This is tricky though. Some cases: - # * lo = 0, hi = 99, extent = 1000 - # We look at hi - lo + 1; resize extent down to 100 - # * lo = 1000, hi = 1099, extent = 1000 - # We look at hi - lo + 1; resize extent down to 100 - # * lo = min for datatype, hi = max for datatype - # We get integer overflow trying to compute hi - lo + 1 - # So if lo <= 0 and hi >= ind_ext, this is fine without - # computing hi - lo + 1. - lo <- ind_max_dom[[1]] - hi <- ind_max_dom[[2]] - if (lo > 0 || hi < ind_ext) { - dom_span <- hi - lo + 1 - if (ind_ext > dom_span) { - ind_ext <- dom_span - } - } - aa <- arrow::arrow_array(c(ind_max_dom, ind_ext, ind_cur_dom), ind_col_type) + arrow::arrow_array(c("", "", "", requested_slot[[1]], requested_slot[[2]]), ind_col_type) } } else { - if (ind_col_type_name %in% c("string", "utf8", "large_utf8")) { - aa <- arrow::arrow_array(c("", "", ""), ind_col_type) - } else { - # Same comments as above - lo <- ind_cur_dom[[1]] - hi <- ind_cur_dom[[2]] - if (lo > 0 || hi < ind_ext) { - dom_span <- hi - lo + 1 - if (ind_ext > dom_span) { - ind_ext <- dom_span - } + # If they wanted (0, 99) then extent must be at most 100. + # This is tricky though. Some cases: + # * lo = 0, hi = 99, extent = 1000 + # We look at hi - lo + 1; resize extent down to 100 + # * lo = 1000, hi = 1099, extent = 1000 + # We look at hi - lo + 1; resize extent down to 100 + # * lo = min for datatype, hi = max for datatype + # We get integer overflow trying to compute hi - lo + 1 + # So if lo <= 0 and hi >= ind_ext, this is fine without + # computing hi - lo + 1. + lo <- ind_max_dom[[1]] + hi <- ind_max_dom[[2]] + if (lo > 0 || hi < ind_ext) { + dom_span <- hi - lo + 1 + if (ind_ext > dom_span) { + ind_ext <- dom_span } - aa <- arrow::arrow_array(c(ind_cur_dom, ind_ext), ind_col_type) } + aa <- arrow::arrow_array(c(ind_max_dom, ind_ext, ind_cur_dom), ind_col_type) } aa @@ -601,7 +577,7 @@ get_domain_and_extent_array <- function(shape, is_sparse) { # expansion. ind_max_dom <- arrow_type_unsigned_range(ind_col_type) - c(0, ind_ext) - if (.new_shape_feature_flag_is_enabled() && (is_sparse || .dense_arrays_can_have_current_domain())) { + if (is_sparse || .dense_arrays_can_have_current_domain()) { aa <- arrow::arrow_array(c(ind_max_dom, ind_ext, ind_cur_dom), ind_col_type) } else { aa <- arrow::arrow_array(c(ind_cur_dom, ind_ext), ind_col_type) diff --git a/apis/r/R/write_soma.R b/apis/r/R/write_soma.R index d0c1106d3e..673dc694ad 100644 --- a/apis/r/R/write_soma.R +++ b/apis/r/R/write_soma.R @@ -225,15 +225,12 @@ write_soma.data.frame <- function( # dataframe with something users think of as a "shape". For the # other slots, set the domain wide open. # - domain <- NULL - if (.new_shape_feature_flag_is_enabled()) { - domain <- list() - for (index_column_name in index_column_names) { - if (index_column_name == "soma_joinid") { - domain[["soma_joinid"]] <- c(0, nrow(x) - 1) - } else { - domain[[index_column_name]] <- NULL - } + domain <- list() + for (index_column_name in index_column_names) { + if (index_column_name == "soma_joinid") { + domain[["soma_joinid"]] <- c(0, nrow(x) - 1) + } else { + domain[[index_column_name]] <- NULL } } diff --git a/apis/r/tests/testthat/test-SOMASparseNDArray.R b/apis/r/tests/testthat/test-SOMASparseNDArray.R index f18a9c24a8..0b32da85a4 100644 --- a/apis/r/tests/testthat/test-SOMASparseNDArray.R +++ b/apis/r/tests/testthat/test-SOMASparseNDArray.R @@ -56,18 +56,9 @@ test_that("SOMASparseNDArray creation", { expect_true(tiledb::is.sparse(sch)) expect_false(tiledb::allows_dups(sch)) - ## shape expect_equal(ndarray$shape(), as.integer64(c(10, 10))) - ## maxshape - # TODO: more testing with current-domain feature integrated - # https://github.com/single-cell-data/TileDB-SOMA/issues/2407 - - if (.new_shape_feature_flag_is_enabled()) { - expect_true(ndarray$tiledbsoma_has_upgraded_shape()) - } else { - expect_false(ndarray$tiledbsoma_has_upgraded_shape()) - } + expect_true(ndarray$tiledbsoma_has_upgraded_shape()) shape <- ndarray$shape() maxshape <- ndarray$maxshape() expect_equal(length(shape), length(maxshape)) diff --git a/apis/r/tests/testthat/test-shape.R b/apis/r/tests/testthat/test-shape.R index d66be1d8af..0f142a0f7b 100644 --- a/apis/r/tests/testthat/test-shape.R +++ b/apis/r/tests/testthat/test-shape.R @@ -56,11 +56,7 @@ test_that("SOMADataFrame shape", { sdf <- SOMADataFrameOpen(uri) # Check shape and maxshape et al. - if (!.new_shape_feature_flag_is_enabled()) { - expect_false(sdf$tiledbsoma_has_upgraded_domain()) - } else { - expect_true(sdf$tiledbsoma_has_upgraded_domain()) - } + expect_true(sdf$tiledbsoma_has_upgraded_domain()) expect_error(sdf$shape(), class = "notYetImplementedError") expect_error(sdf$maxshape(), class = "notYetImplementedError") @@ -83,11 +79,7 @@ test_that("SOMADataFrame shape", { } # Check has_upgraded_domain - if (!.new_shape_feature_flag_is_enabled()) { - expect_false(sdf$tiledbsoma_has_upgraded_domain()) - } else { - expect_true(sdf$tiledbsoma_has_upgraded_domain()) - } + expect_true(sdf$tiledbsoma_has_upgraded_domain()) # Check domain and maxdomain dom <- sdf$domain() @@ -132,11 +124,6 @@ test_that("SOMADataFrame shape", { sjid_mxd <- mxd[["soma_joinid"]] sjid_dfc <- domain_for_create[["soma_joinid"]] - if (!.new_shape_feature_flag_is_enabled()) { - # Old behavior - expect_equal(sjid_dom, sjid_mxd) - } - # Not: expect_equal(sjid_dom, bit64::as.integer64(sjid_dfc)) The # soma_joinid dim is always of type int64. Everything coming back # from libtiledbsoma, through C nanoarrow, through the R arrow @@ -155,19 +142,10 @@ test_that("SOMADataFrame shape", { int_mxd <- mxd[["int_column"]] int_dfc <- domain_for_create[["int_column"]] - if (!.new_shape_feature_flag_is_enabled()) { - # Old behavior - expect_equal(int_dom, int_mxd) - } - expect_equal(int_dom, int_dfc) - if (!.new_shape_feature_flag_is_enabled()) { - expect_equal(int_mxd, int_dfc) - } else { - expect_true(int_mxd[[1]] < -2000000000) - expect_true(int_mxd[[2]] > 2000000000) - } + expect_true(int_mxd[[1]] < -2000000000) + expect_true(int_mxd[[2]] > 2000000000) } if ("string_column" %in% index_column_names) { @@ -175,84 +153,78 @@ test_that("SOMADataFrame shape", { str_mxd <- mxd[["string_column"]] str_dfc <- domain_for_create[["string_column"]] - if (!.new_shape_feature_flag_is_enabled()) { + if (is.null(str_dfc)) { expect_equal(str_dom, c("", "")) - expect_equal(str_mxd, c("", "")) } else { - if (is.null(str_dfc)) { - expect_equal(str_dom, c("", "")) - } else { - expect_equal(str_dom, str_dfc) - } - expect_equal(str_mxd, c("", "")) + expect_equal(str_dom, str_dfc) } + expect_equal(str_mxd, c("", "")) } sdf$close() # Test resize for dataframes (more general upgrade_domain to be tested # separately -- see https://github.com/single-cell-data/TileDB-SOMA/issues/2407) - if (.new_shape_feature_flag_is_enabled()) { - has_soma_joinid_dim <- "soma_joinid" %in% index_column_names - sjid_dfc <- domain_for_create[["soma_joinid"]] + has_soma_joinid_dim <- "soma_joinid" %in% index_column_names + sjid_dfc <- domain_for_create[["soma_joinid"]] - # Test resize down - new_shape <- 0 - sdf <- SOMADataFrameOpen(uri, "WRITE") - if (has_soma_joinid_dim) { - # It's an error to downsize - expect_error(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape)) - } else { - # There is no problem when soma_joinid is not a dim -- - # sdf$tiledbsoma_resize_soma_joinid_shape is a no-op in that case - expect_no_condition(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape)) - } - sdf$close() - - # Make sure the failed resize really didn't change the shape - if (has_soma_joinid_dim) { - sdf <- SOMADataFrameOpen(uri, "READ") - expect_equal(sdf$domain()[["soma_joinid"]], sjid_dfc) - sdf$close() - } + # Test resize down + new_shape <- 0 + sdf <- SOMADataFrameOpen(uri, "WRITE") + if (has_soma_joinid_dim) { + # It's an error to downsize + expect_error(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape)) + } else { + # There is no problem when soma_joinid is not a dim -- + # sdf$tiledbsoma_resize_soma_joinid_shape is a no-op in that case + expect_no_condition(sdf$tiledbsoma_resize_soma_joinid_shape(new_shape)) + } + sdf$close() - # Test writes out of bounds, before resize - old_shape <- 100 - if (has_soma_joinid_dim) { - old_shape <- domain_for_create[["soma_joinid"]][[2]] + 1 + 100 - } - new_shape <- old_shape + 100 - - tbl1 <- arrow::arrow_table( - int_column = 5L:8L, - soma_joinid = (old_shape + 1L):(old_shape + 4L), - float_column = 5.1:8.1, - string_column = c("egg", "flag", "geese", "hay"), - schema = asch - ) - - sdf <- SOMADataFrameOpen(uri, "WRITE") - if (has_soma_joinid_dim) { - expect_error(sdf$write(tbl1)) - } else { - expect_no_condition(sdf$write(tbl1)) - } + # Make sure the failed resize really didn't change the shape + if (has_soma_joinid_dim) { + sdf <- SOMADataFrameOpen(uri, "READ") + expect_equal(sdf$domain()[["soma_joinid"]], sjid_dfc) sdf$close() + } - # Test resize - sdf <- SOMADataFrameOpen(uri, "WRITE") - sdf$tiledbsoma_resize_soma_joinid_shape(new_shape) - sdf$close() + # Test writes out of bounds, before resize + old_shape <- 100 + if (has_soma_joinid_dim) { + old_shape <- domain_for_create[["soma_joinid"]][[2]] + 1 + 100 + } + new_shape <- old_shape + 100 - # Test writes out of old bounds, within new bounds, after resize - sdf <- SOMADataFrameOpen(uri, "WRITE") + tbl1 <- arrow::arrow_table( + int_column = 5L:8L, + soma_joinid = (old_shape + 1L):(old_shape + 4L), + float_column = 5.1:8.1, + string_column = c("egg", "flag", "geese", "hay"), + schema = asch + ) + + sdf <- SOMADataFrameOpen(uri, "WRITE") + if (has_soma_joinid_dim) { + expect_error(sdf$write(tbl1)) + } else { expect_no_condition(sdf$write(tbl1)) - sdf$close() + } + sdf$close() - # To do: test readback + # Test resize + sdf <- SOMADataFrameOpen(uri, "WRITE") + sdf$tiledbsoma_resize_soma_joinid_shape(new_shape) + sdf$close() + + # Test writes out of old bounds, within new bounds, after resize + sdf <- SOMADataFrameOpen(uri, "WRITE") + expect_no_condition(sdf$write(tbl1)) + sdf$close() + + # To do: test readback + + rm(tbl1) - rm(tbl1) - } rm(sdf, tbl0) @@ -342,8 +314,6 @@ test_that("SOMADataFrame shape", { }) test_that("SOMADataFrame domain mods", { - skip_if(!.new_shape_feature_flag_is_enabled()) - uri <- withr::local_tempdir("soma-dataframe-domain-mods") schema = arrow::schema( @@ -465,11 +435,7 @@ test_that("SOMASparseNDArray shape", { readback_shape <- ndarray$shape() readback_maxshape <- ndarray$maxshape() expect_equal(length(readback_shape), length(readback_maxshape)) - if (.new_shape_feature_flag_is_enabled()) { - expect_true(all(readback_shape < readback_maxshape)) - } else { - expect_true(all(readback_shape == readback_maxshape)) - } + expect_true(all(readback_shape < readback_maxshape)) ndarray$close() @@ -496,38 +462,36 @@ test_that("SOMASparseNDArray shape", { ndarray$close() - if (.new_shape_feature_flag_is_enabled()) { - ndarray <- SOMASparseNDArrayOpen(uri, "WRITE") + ndarray <- SOMASparseNDArrayOpen(uri, "WRITE") - # Test resize down - new_shape <- c(50, 60) - expect_error(ndarray$resize(new_shape)) + # Test resize down + new_shape <- c(50, 60) + expect_error(ndarray$resize(new_shape)) - # Test writes out of old bounds - soma_dim_0 <- c(200, 300) - soma_dim_1 <- c(400, 500) - soma_data <- c(6000, 7000) - sm <- sparseMatrix(i = soma_dim_0, j = soma_dim_1, x = soma_data) - expect_error(ndarray$write(sm)) - - # Test resize up - new_shape <- c(500, 600) - #### expect_no_error(ndarray$resize(new_shape)) - ndarray$resize(new_shape) - - # Test writes within new bounds - soma_dim_0 <- c(200, 300) - soma_dim_1 <- c(400, 500) - soma_data <- c(6000, 7000) - sm <- sparseMatrix(i = soma_dim_0, j = soma_dim_1, x = soma_data) - expect_no_error(ndarray$write(sm)) - ndarray$close() - - ndarray <- SOMASparseNDArrayOpen(uri) - coords <- list(bit64::as.integer64(c(101, 202)), bit64::as.integer64(c(3, 4))) - expect_no_error(x <- ndarray$read(coords = coords)$tables()$concat()) - ndarray$close() - } + # Test writes out of old bounds + soma_dim_0 <- c(200, 300) + soma_dim_1 <- c(400, 500) + soma_data <- c(6000, 7000) + sm <- sparseMatrix(i = soma_dim_0, j = soma_dim_1, x = soma_data) + expect_error(ndarray$write(sm)) + + # Test resize up + new_shape <- c(500, 600) + #### expect_no_error(ndarray$resize(new_shape)) + ndarray$resize(new_shape) + + # Test writes within new bounds + soma_dim_0 <- c(200, 300) + soma_dim_1 <- c(400, 500) + soma_data <- c(6000, 7000) + sm <- sparseMatrix(i = soma_dim_0, j = soma_dim_1, x = soma_data) + expect_no_error(ndarray$write(sm)) + ndarray$close() + + ndarray <- SOMASparseNDArrayOpen(uri) + coords <- list(bit64::as.integer64(c(101, 202)), bit64::as.integer64(c(3, 4))) + expect_no_error(x <- ndarray$read(coords = coords)$tables()$concat()) + ndarray$close() rm(ndarray) gc() @@ -556,12 +520,8 @@ test_that("SOMADenseNDArray shape", { readback_maxshape <- ndarray$maxshape() expect_equal(length(readback_shape), length(readback_maxshape)) - if (.new_shape_feature_flag_is_enabled()) { - if (.dense_arrays_can_have_current_domain()) { - expect_true(all(readback_shape < readback_maxshape)) - } else { - expect_true(all(readback_shape == readback_maxshape)) - } + if (.dense_arrays_can_have_current_domain()) { + expect_true(all(readback_shape < readback_maxshape)) } else { expect_true(all(readback_shape == readback_maxshape)) } @@ -591,46 +551,44 @@ test_that("SOMADenseNDArray shape", { ndarray$close() - if (.new_shape_feature_flag_is_enabled()) { - ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") + ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") - # Test resize down - new_shape <- c(50, 60) - expect_error(ndarray$resize(new_shape)) + # Test resize down + new_shape <- c(50, 60) + expect_error(ndarray$resize(new_shape)) - # Test writes out of old bounds - ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") - mat <- create_dense_matrix_with_int_dims(300, 400) - expect_error(ndarray$write(mat)) - ndarray$close() + # Test writes out of old bounds + ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") + mat <- create_dense_matrix_with_int_dims(300, 400) + expect_error(ndarray$write(mat)) + ndarray$close() - # Test resize up - new_shape <- c(500, 600) - if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { - expect_no_error(ndarray$resize(new_shape)) - } else { - expect_error(ndarray$resize(new_shape)) - } + # Test resize up + new_shape <- c(500, 600) + if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { + expect_no_error(ndarray$resize(new_shape)) + } else { + expect_error(ndarray$resize(new_shape)) + } - # Test writes within new bounds - ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") - mat <- create_dense_matrix_with_int_dims(500, 600) - if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { - expect_no_error(ndarray$write(mat)) - } else { - expect_error(ndarray$write(mat)) - } - ndarray$close() + # Test writes within new bounds + ndarray <- SOMADenseNDArrayOpen(uri, "WRITE") + mat <- create_dense_matrix_with_int_dims(500, 600) + if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { + expect_no_error(ndarray$write(mat)) + } else { + expect_error(ndarray$write(mat)) + } + ndarray$close() - ndarray <- SOMADenseNDArrayOpen(uri) - coords <- list(bit64::as.integer64(c(101, 202)), bit64::as.integer64(c(3, 4))) - if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { - expect_no_condition(x <- ndarray$read_dense_matrix(coords = coords)) - } else { - expect_error(x <- ndarray$read(coords = coords)$tables()$concat()) - } - ndarray$close() + ndarray <- SOMADenseNDArrayOpen(uri) + coords <- list(bit64::as.integer64(c(101, 202)), bit64::as.integer64(c(3, 4))) + if (tiledbsoma:::.dense_arrays_can_have_current_domain()) { + expect_no_condition(x <- ndarray$read_dense_matrix(coords = coords)) + } else { + expect_error(x <- ndarray$read(coords = coords)$tables()$concat()) } + ndarray$close() rm(ndarray) gc() diff --git a/apis/r/tests/testthat/test-write-soma-resume.R b/apis/r/tests/testthat/test-write-soma-resume.R index e16f00aa80..116365909d 100644 --- a/apis/r/tests/testthat/test-write-soma-resume.R +++ b/apis/r/tests/testthat/test-write-soma-resume.R @@ -163,10 +163,8 @@ test_that("Resume-mode data frames", { } } - if (.new_shape_feature_flag_is_enabled()) { - sdfp$reopen("WRITE") - sdfp$tiledbsoma_resize_soma_joinid_shape(nrow(co2)) - } + sdfp$reopen("WRITE") + sdfp$tiledbsoma_resize_soma_joinid_shape(nrow(co2)) expect_s3_class( sdfc <- write_soma(