Failed delete propagation through PositionGroup

[samuelbray32]

Describe the bug

• PositionGroup has a part table PositionGroup.Position which is dependent on PositionOutput
• When a delete in PositionOutput or above tries to pass through this, it fails due to trying to delete from part before master.
• This is similar to the issue with merge tables, but since PositionOutput isn't a merge table, the solution isn't applied.

Potential Solutions
There may be a simpler way to fix, but one idea would be to have a superclass Group similar to Merge that lets us resolve this type of dependency.

To Reproduce

from spyglass.position.v1 import TrodesPosV1
from spyglass.position import PositionOutput
from spyglass.linearization.merge import LinearizedPositionOutput
from spyglass.decoding.v1.sorted_spikes import PositionGroup
key = "nwb_file_name LIKE 'Bilbo%'"
print(len(TrodesPosV1() & key))
(TrodesPosV1() & key).delete()

Error

Details

{
	"name": "DataJointError",
	"message": "Attempt to delete part table `decoding_core_v1`.`position_group__position` before deleting from its master `decoding_core_v1`.`position_group` first.",
	"stack": "---------------------------------------------------------------------------
DataJointError                            Traceback (most recent call last)
Untitled-1.ipynb Cell 1 line 8
      <a href='vscode-notebook-cell:Untitled-1.ipynb?jupyter-notebook#W0sdW50aXRsZWQ%3D?line=5'>6</a> key = \"nwb_file_name LIKE 'Bilbo%'\"
      <a href='vscode-notebook-cell:Untitled-1.ipynb?jupyter-notebook#W0sdW50aXRsZWQ%3D?line=6'>7</a> print(len(TrodesPosV1() & key))
----> <a href='vscode-notebook-cell:Untitled-1.ipynb?jupyter-notebook#W0sdW50aXRsZWQ%3D?line=7'>8</a> (TrodesPosV1() & key).delete()

File ~/Documents/spyglass/src/spyglass/utils/dj_mixin.py:489, in SpyglassMixin.delete(self, force_permission, *args, **kwargs)
    487 def delete(self, force_permission=False, *args, **kwargs):
    488     \"\"\"Alias for cautious_delete, overwrites datajoint.table.Table.delete\"\"\"
--> 489     self.cautious_delete(force_permission=force_permission, *args, **kwargs)

File ~/Documents/spyglass/src/spyglass/utils/dj_mixin.py:473, in SpyglassMixin.cautious_delete(self, force_permission, *args, **kwargs)
    467     dj_logger.info(f\"Merge: Deleting {count} rows from {table}\")
    468 if (
    469     not self._test_mode
    470     or not safemode
    471     or user_choice(\"Commit deletes?\", default=\"no\") == \"yes\"
    472 ):
--> 473     self._commit_merge_deletes(merge_deletes, **kwargs)
    474 else:
    475     logger.info(\"Delete aborted.\")

File ~/Documents/spyglass/src/spyglass/utils/dj_mixin.py:208, in SpyglassMixin._commit_merge_deletes(self, merge_join_dict, **kwargs)
    206 table = self._merge_tables[table_name]
    207 keys = [part.fetch(MERGE_PK, as_dict=True) for part in part_restr]
--> 208 (table & keys).delete(**kwargs)

File ~/mambaforge-pypy3/envs/spyglass/lib/python3.9/site-packages/datajoint/table.py:601, in Table.delete(self, transaction, safemode, force_parts)
    599             if transaction:
    600                 self.connection.cancel_transaction()
--> 601             raise DataJointError(
    602                 \"Attempt to delete part table {part} before deleting from \"
    603                 \"its master {master} first.\".format(part=part, master=master)
    604             )
    606 # Confirm and commit
    607 if delete_count == 0:

DataJointError: Attempt to delete part table `decoding_core_v1`.`position_group__position` before deleting from its master `decoding_core_v1`.`position_group` first."
}

<\details>

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

[samuelbray32]

Not urgent for progress since can get around it by deleting from PositionGroup first and then running the delete command above

[CBroz1]

This raises a question for me of what delete_downstream_merge should do. I think this case is an example of a false positive in the existing code: it looks like a merge table, but isn't, so the delete doesn't work. In an ideal implementation of my current strategy, this would not have caught this table.

The feature of merge tables that requires this process is part table foreign key inheritance, which this table shares. From here, we could either decide...

1. Non-merge tables with this feature should throw the 'part-before-master' error.
2. All Spyglass tables with this feature should delete their master entries, and never hit the above error.

If (2), the solution in #899 should apply to all parts in spyglass, essentially adding a feature to datajoint part tables. The downside would be potentially queuing unexpected items for deletion.

What do you think, @samuelbray32 @edeno ?

[edeno]

I mean I'm a fan of anything that makes it easier for the users, even it is a departure from datajoint, so I would vote 2. Do you happen to know of any reasons to not do 2? or reasons on the datajoint philosophy for not allowing 2?

[CBroz1]

No hard reason not to. It is coincidentally explicitly discussed as Option 2 here.

[edeno]

Yeah, I think we should go ahead and do it and not wait for datajoint. Perhaps we can contribute back or make it a more urgent issue for them in the future.

[CBroz1]

Awaiting feedback on proposed edit to datajoint here