Multi table #410
Merged
Multi table #410
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 23.10.1 → 23.11.0](psf/black@23.10.1...23.11.0) - [github.com/pre-commit/mirrors-prettier: v3.0.3 → v3.1.0](pre-commit/mirrors-prettier@v3.0.3...v3.1.0) - [github.com/pre-commit/mirrors-mypy: v1.6.1 → v1.7.0](pre-commit/mirrors-mypy@v1.6.1...v1.7.0) - [github.com/astral-sh/ruff-pre-commit: v0.1.3 → v0.1.6](astral-sh/ruff-pre-commit@v0.1.3...v0.1.6) * ficx pre-precommit --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: giovp <[email protected]>
for more information, see https://pre-commit.ci
…into multi_table
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…into multi_table
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.11 → v0.1.13](astral-sh/ruff-pre-commit@v0.1.11...v0.1.13) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…erse#427) * made _locate_spatial_element public, renamed to locate_element() * returning path instead of tuple in locate_element() * updated changelog * locate_elements() now returns a list
Triggering the tests for pull requests to any branch.
|
Merging as agreed changes have been implemented and checked. |
|
amazing work @melonora ! |
melonora
added a commit
that referenced
this pull request
Mar 14, 2024
* initial tests multi_table design (#405) * initial tests multi_table design * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add mock new table * create test class and cleanup * additional cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * additional cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add pseudo methods --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Multi table (#410) * [pre-commit.ci] pre-commit autoupdate (#394) * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 23.10.1 → 23.11.0](psf/black@23.10.1...23.11.0) - [github.com/pre-commit/mirrors-prettier: v3.0.3 → v3.1.0](pre-commit/mirrors-prettier@v3.0.3...v3.1.0) - [github.com/pre-commit/mirrors-mypy: v1.6.1 → v1.7.0](pre-commit/mirrors-mypy@v1.6.1...v1.7.0) - [github.com/astral-sh/ruff-pre-commit: v0.1.3 → v0.1.6](astral-sh/ruff-pre-commit@v0.1.3...v0.1.6) * ficx pre-precommit --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: giovp <[email protected]> * initial tests multi_table design * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add mock new table * create test class and cleanup * additional cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * additional cleanup * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add pseudo methods * Change table type in init * make tables plural and add to validation in __init__ * revert to old public accessor * Validate each table in dictionary * iterate dict values * add comment * adjust table getter * Add tables getter * Fix missing parenthesis * change to warnings.warn DeprecationWarning * allow for backward compatibility in init * [pre-commit.ci] pre-commit autoupdate (#408) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix dict subscriptable * fix string representation of sdata * add deprecation decorator for future * Allow for tables not annotating elements * switch to using tables with deprecation * fix string representation * write tables element group * adjust io to multi_table * Alter io to give None as default value for spatialdata attrs keys * add tables setter * raise keyerror table getter * remove commented tables setter * raise keyerror in table deleter * add deprecation warning * fix tests * add DeprecationWarning * comment test * change setter into method * circumvent mappingproxy set issue * adjust set get test * add get table keys * add column getters * add change set target table * Give default table name * fix spatialdata without table * add int32 because of windows and add docstring * fix filtering by coordinate system * Change to Path to not be linux / mac specific * Change to Path to not be linux / mac specific * table should annotate existing element * return table with AnnData having 0 rows * Adjust for windows * adjust for accessing table elements * fix change annotation target * fix set annotation target * fix/add tests * fix init from elements * fix init from elements tests * add validation check * add table validation SpatialData.__init * fix ruff * only concatenate if annotating * change into warning because of filtering * fix last tests * adjust to tables * use tables parameter * fix some mypy * some mypy fixes * some more mypy * fix another mypy * circumvent typing error on py3.9 * mypy yet again * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix pre_commit * down to 12 mypy errors * down to 1mypy error * fixed mypy errors * fix set_table_annotation * added docstring * refactor data loader (#299) Co-authored-by: LucaMarconato <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Luca Marconato <[email protected]> * add documentation * add documentation * minor adjustment docstring * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added / adjusted docstrings * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix mypy after merge * refactor function name This is to avoid confusion. Many not easily resolved errors are created if we let this function generate table values. This makes it clear that only spatial element values are generated and not tables. This in opposite to gen_elements which does return tables as well. * [pre-commit.ci] pre-commit autoupdate (#411) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * small fixes * added gen_elements docstrings * tiny comments * fix ruff pre-commit * removed types from docstring * refactor of set_table_annotation_target * add quotes * fix (?) * refactor error messages * fix incremental update (#329) Co-authored-by: Wouter-Michiel Vierdag <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * add concatenate argument * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add util functions to init * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add util functions to init * add tables class * add table class * add deprecation back * rename function in tests * rename function in tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix precommit * update precommit and remove add_table, store_table and general fixes * adjust tables init to incremental update pr * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix deletion of deprecated table * revert filter change * add public generators * adjust to public generators * Find element uses public generator * add validation in sdata for tables * add deprecation version number * fix mypy errors * Fix backing when deleting table * Fix mypy * cleanup * chance target_element_name to region * refactor test * adjust concatenate regarding not concatenating tables * add utility function * concatenate if in multiple sdata objects * minor docstring refactor * fix import * concatenate with tables * cleanup * fix test * [pre-commit.ci] pre-commit autoupdate (#415) updates: - [github.com/psf/black: 23.11.0 → 23.12.1](psf/black@23.11.0...23.12.1) - [github.com/pre-commit/mirrors-prettier: v4.0.0-alpha.4 → v4.0.0-alpha.8](pre-commit/mirrors-prettier@v4.0.0-alpha.4...v4.0.0-alpha.8) - [github.com/pre-commit/mirrors-mypy: v1.7.1 → v1.8.0](pre-commit/mirrors-mypy@v1.7.1...v1.8.0) - [github.com/astral-sh/ruff-pre-commit: v0.1.7 → v0.1.9](astral-sh/ruff-pre-commit@v0.1.7...v0.1.9) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fixed typings; made pytest raises explicit * minor fixes human readable strings * fixed tests * fix * Add changes to changelog * make private * Remove commented code * add cache to ignore * updated changelog with giovp old pr * refactor into private function * Fix docstring * Fix import * specify key reuse in docstring * add orphan_table argument * Change docstring * remove todo * add example * change concatenate logic * updated changelog * Allow force-overwriting existing files (non-backing) (#344) * Add test for writing unbacked data over existing files * Protect overwriting existing file only if it is backing file * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Simplify assertion, remove try/except * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed pre-commit * added get_dask_backing_files(); improved sdata.write with overwrite=True * fix docs * changed version in changelog * fix exception string --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Luca Marconato <[email protected]> Co-authored-by: LucaMarconato <[email protected]> * Added error message for removed add_elements functions (#420) * added error message for removed add_elements functions * moved _error_message_add_element() to _utils * added validate and set region key * fix docstring * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added public functions Spatialdata * fix tests * add docstrings * Added subset API; fix behavior with zero-len table (#426) * added subset API, returning None instead of empty table for APIs with filter_table=True * fix 3.9 * [pre-commit.ci] pre-commit autoupdate (#424) updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.9 → v0.1.11](astral-sh/ruff-pre-commit@v0.1.9...v0.1.11) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * remove docstring typehint * Warn user over overwrite in docstring * Fix query of 2D/3D data with 2D/3D bounding box (#409) * wip * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * wip * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix 2d/3d bb for raster data * support for 2d/3d bb for 2d/3d points * better tests * applied suggestions from giovp * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * added comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * [pre-commit.ci] pre-commit autoupdate (#430) updates: - [github.com/astral-sh/ruff-pre-commit: v0.1.11 → v0.1.13](astral-sh/ruff-pre-commit@v0.1.11...v0.1.13) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * updated changelog * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor subset * remove todo * Made _locate_spatial_element public, renamed to locate_element() (#427) * made _locate_spatial_element public, renamed to locate_element() * returning path instead of tuple in locate_element() * updated changelog * locate_elements() now returns a list * fix test * Update test_and_deploy.yaml (#434) Triggering the tests for pull requests to any branch. * change docstring * fix query test * add todo * refactor filter_by_coordinate_system * test filter with keep table * adjust docstring * adjust docstring --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: giovp <[email protected]> Co-authored-by: Giovanni Palla <[email protected]> Co-authored-by: LucaMarconato <[email protected]> Co-authored-by: Luca Marconato <[email protected]> Co-authored-by: aeisenbarth <[email protected]> * Enforce instance key to be dtype int (#444) * change genes in blobs points * force instance_id of int dtype * Change error * Check unique instance_key values per region * Join elements table (#445) * change genes in blobs points * force instance_id of int dtype * Change error * Check unique instance_key values per region * silence warning * add left inner join * change to left join * add left_exclusive join * add to Enum * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add inner join * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add right join * add ugly right exclusive join * return none instead of empty df * add left join tests * test left_exclusive_join * test inner join * refactor get_table_keys * assert valid element in elements_dict * test warnings * add fail join tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * silence warning * add left inner join * change to left join * add left_exclusive join * add to Enum * add inner join * fix merge conflict * Add right join * add ugly right exclusive join * return none instead of empty df * add left join tests * test left_exclusive_join * test inner join * refactor get_table_keys * assert valid element in elements_dict * test warnings * add fail join tests * fix comments * lowercase + docstring * complete docstring * lowercase enum * explicit instance_id column in test * test instance_id and region column * change error * add get method * add contain * get rid of create_element_dict * add todo * adjust enums * add ability for matching rows * some cleanup * make any element none in right exclusive join * add check of table order conservation * fix tests * add match_element_to_table * change docstring * revert to old match_table_to_element * readd match_element_to_table * added tests for label joins * add points tests * fixed type and docstring * fixed comments and docstrings * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * change typehint * update changelog with joins * include join functions * docs for joins * added test for right match rows * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * add validation of match rows * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix docstring * fix docs * fix typo in changelog * attempt fix docs * fix docs * fix docs --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Luca Marconato <[email protected]> Co-authored-by: LucaMarconato <[email protected]> * fix tests * fix docs * add possibility for custom table name (#459) * add possibility for custom table name * change docstring * updated changelog * added table_name to SpatialData.aggregate() --------- Co-authored-by: Luca Marconato <[email protected]> * Update locate values (#460) * add possibility for custom table name * change docstring * updated changelog * added table_name parameter * update changelog --------- Co-authored-by: LucaMarconato <[email protected]> * Filter table annotate (#462) * add get_element_annotator * add docstring * add test * wip get_centroids * implemented get_centroids() * made _assert_spatialdata_objects_seem_identical() into a util * fix docs, attemp * allow table to be None in get_values and _locate_values (#466) * allow table to be None * fixes to aggregate * check annotation * adjust docstrings * remove table check * add sphinx_pytest * Added `validate_table_annotation_target()` (#468) * added validate_table_annotation_target() * removed test that is no longer relevant * fix docs * fixed way to test no warnings emitted * merged validate_table_annotation_target() into validate_table_in_spatialdata() * fix test * silence warning * fix gettin dtype from multiscale * add else dtype back * silence scipy.misc.face deprecation * Operation `to_circles()` (#473) * `get_centroids()` (#465) * wip get_centroids * implemented get_centroids() * code suggestions from kevin * wip vectorize.py * added vectorize; still no tests * refactored testing functions; wip tests to_circles() * considering removing the target_coordinate_system parameter in to_circles() * adjusted tests, docs, changelog * added changelog * fix tests, remove inject_docs * fix sphinx * attempt fix sphinx * deedcopy() utils function (#480) * deedcopy() utils function * fixed missings seeds for default_rng() * wip fix * wip fix * fix bug due to data being computed in-place and then failing validation * add pooch requirement * Update src/spatialdata/_core/_deepcopy.py * Update src/spatialdata/_core/_deepcopy.py * Update src/spatialdata/_core/_deepcopy.py * Update src/spatialdata/_core/_deepcopy.py --------- Co-authored-by: Wouter-Michiel Vierdag <[email protected]> * fix bug deepcopy() of points wrong columns order * workaround wrong order points columns after deepcopy * rechunking raster data after spatial query (#479) * rechunking raster data after spatial query * using xarray chunk() instead of dask rechunk() * Test joins with string indices and instance id (#485) * test join strings * fix dtype aggregate --------- Co-authored-by: Luca Marconato <[email protected]> * cleanup tests * remove comments --------- Co-authored-by: Wouter-Michiel Vierdag <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: giovp <[email protected]> Co-authored-by: Giovanni Palla <[email protected]> Co-authored-by: aeisenbarth <[email protected]> Co-authored-by: Wouter-Michiel Vierdag <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the PR adding support for multiple tables in SpatialData. Tables still use
region,region_keyandinstance_keyintable.uns['spatialdata_attrs']. However, all of these need to be present or none of these are present. This supports the case of tables not annotating an element. Basic functionality is as shown intest_multiple_table.py.Two open issues are what the interface should be for concatenating SpatialData objects. The current implementation assumes only 1 table per SpatialData object to be present with only one name 'table'. This is not the case anymore. I propose two options.
Personally I would vote for the latter option.
The other issue is regarding the dataloader still assuming 1 table. Question here is whether we still only want to support 1 table for each dataloader instance of whether we want multiple. My proposal would be to have an argument 'table_name' specifying which table should be in the dataloader. Question here is whether there would be a usecase for having more than 1 table in the dataloader.
Matching elements to table and vice versa still needs to be better supported, but given the size of the PR I would reserve this for the next PR. Also reserved for a follow up PR would be the filtering of specific tables if the user wishes to do so.
Note:
The function 'gen_elements' iterates also over tables. In order to avoid confusion with 'gen_elements_values' I have renamed the latter to 'gen_spatial_elements_values' as the latter is often assigned to a variable and does not allow for filtering out tables.