Conversation
* [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 <giov.pll@gmail.com>
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>
…into multi_table
Co-authored-by: Wouter-Michiel Vierdag <w-mv@hotmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…into multi_table
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…into multi_table
for more information, see https://pre-commit.ci
|
In the other PR there is currently 1 remaining errors which relates to removing the |
|
@melonora can you please check if this branch can merged/removed? |
| # filtering with tables having potentially different keys. | ||
| if filter_tables: | ||
| tables = {} | ||
| tables: Tables = Tables(shared_keys=self._shared_keys) |
There was a problem hiding this comment.
comparing this to #410 there is a difference here. However, I am not certain that shared_keys needs to be passed here since we are returning a new SpatialData object. What do you think @giovp @LucaMarconato ?
There was a problem hiding this comment.
yeah I agree, the shared keys should be instantiated in the return line 680. I think this can be closed!
There was a problem hiding this comment.
After offline conversation with Giovanni, we agreed it is not required.
|
I will close this PR as all the changes have already been implemented in #410 |
3 participants
@melonora these are some additional fixes, I think we need to remove the add_table/store_table etc. due to the merge of #329 and keep the Tables element as close as possible to the rest.