Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge key conflict in PositionOutput #915

Closed
samuelbray32 opened this issue Apr 4, 2024 · 3 comments
Closed

Merge key conflict in PositionOutput #915

samuelbray32 opened this issue Apr 4, 2024 · 3 comments
Assignees
Labels
bug Something isn't working merge To do with merge tables position

Comments

@samuelbray32
Copy link
Collaborator

Describe the bug
Found a merge id in PositionOutput that has entries in both the CommonPos and TrodesV1 part tables. This prevents accessing the data for the merge id in downstream tables since Merge.merge_get_parents() finds multiple sources.

Relevant notes

To Reproduce
This fails:

pos_key = {"merge_id":'9ba1f7d5-c9a3-b19a-19a6-e92b8674866e'}
(PositionOutput()&pos_key).fetch_nwb(pos_key)

This returns multiple populated parts:

pos_key = {"merge_id":'9ba1f7d5-c9a3-b19a-19a6-e92b8674866e'}
(PositionOutput()&pos_key)._merge_restrict_parts(pos_key)
Error Stack
{
	"name": "ValueError",
	"message": "Found  2 potential parents: [FreeTable(`common_position`.`__interval_position_info`)
*position_info *nwb_file_name *interval_list analysis_file_ head_position_ head_orientati head_velocity_
+------------+ +------------+ +------------+ +------------+ +------------+ +------------+ +------------+
default_decodi j1620210710_.n pos 1 valid ti j1620210710_QI db89f802-fbbb- e637ea9c-7498- 1cd01a9a-dc4c-
 (Total: 1)
, FreeTable(`position_v1_trodes_position`.`__trodes_pos_v1`)
*nwb_file_name *interval_list *trodes_pos_pa analysis_file_ position_objec orientation_ob velocity_objec
+------------+ +------------+ +------------+ +------------+ +------------+ +------------+ +------------+
j1620210710_.n pos 1 valid ti default_decodi j1620210710_3X 007abc39-1b4e- 584206df-fd52- 2e13ef96-5183-
 (Total: 1)
]
\tTry adding a string restriction when invoking `get_parent`. Or permitting multiple sources with `multi_source=True`.",
	"stack": "---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[63], line 5
      3 pos_key = {\"merge_id\":'9ba1f7d5-c9a3-b19a-19a6-e92b8674866e'}
      4 # (PositionOutput()&pos_key)._merge_restrict_parts(pos_key)
----> 5 (PositionOutput()&pos_key).fetch_nwb(pos_key)

File ~/Documents/spyglass/src/spyglass/utils/dj_merge_tables.py:520, in Merge.fetch_nwb(self, restriction, multi_source, disable_warning, *attrs, **kwargs)
    517     raise ValueError(\"Try replacing Merge.method with Merge().method\")
    518 restriction = restriction or self.restriction or True
--> 520 return self.merge_restrict_class(restriction).fetch_nwb()

File ~/Documents/spyglass/src/spyglass/utils/dj_merge_tables.py:710, in Merge.merge_restrict_class(self, key)
    708 def merge_restrict_class(self, key: dict) -> dj.Table:
    709     \"\"\"Returns native parent class, restricted with key.\"\"\"
--> 710     parent_key = self.merge_get_parent(key).fetch(\"KEY\", as_dict=True)
    712     if len(parent_key) > 1:
    713         raise ValueError(
    714             f\"Ambiguous entry. Data has mult rows in parent:\
\\tData:{key}\"
    715             + f\"\
\\t{parent_key}\"
    716         )

File ~/Documents/spyglass/src/spyglass/utils/dj_merge_tables.py:642, in Merge.merge_get_parent(cls, restriction, join_master, multi_source, return_empties, add_invalid_restrict)
    634 part_parents = cls._merge_restrict_parents(
    635     restriction=restriction,
    636     as_objects=True,
    637     return_empties=return_empties,
    638     add_invalid_restrict=add_invalid_restrict,
    639 )
    641 if not multi_source and len(part_parents) != 1:
--> 642     raise ValueError(
    643         f\"Found  {len(part_parents)} potential parents: {part_parents}\"
    644         + \"\
\\tTry adding a string restriction when invoking \"
    645         + \"`get_parent`. Or permitting multiple sources with \"
    646         + \"`multi_source=True`.\"
    647     )
    649 if join_master:
    650     part_parents = [cls * part for part in part_parents]

ValueError: Found  2 potential parents: [FreeTable(`common_position`.`__interval_position_info`)
*position_info *nwb_file_name *interval_list analysis_file_ head_position_ head_orientati head_velocity_
+------------+ +------------+ +------------+ +------------+ +------------+ +------------+ +------------+
default_decodi j1620210710_.n pos 1 valid ti j1620210710_QI db89f802-fbbb- e637ea9c-7498- 1cd01a9a-dc4c-
 (Total: 1)
, FreeTable(`position_v1_trodes_position`.`__trodes_pos_v1`)
*nwb_file_name *interval_list *trodes_pos_pa analysis_file_ position_objec orientation_ob velocity_objec
+------------+ +------------+ +------------+ +------------+ +------------+ +------------+ +------------+
j1620210710_.n pos 1 valid ti default_decodi j1620210710_3X 007abc39-1b4e- 584206df-fd52- 2e13ef96-5183-
 (Total: 1)
]
\tTry adding a string restriction when invoking `get_parent`. Or permitting multiple sources with `multi_source=True`."
}
@samuelbray32 samuelbray32 added bug Something isn't working position merge To do with merge tables labels Apr 4, 2024
@edeno
Copy link
Collaborator

edeno commented Apr 4, 2024

I think this was the very thing that caused the issue to be posted.

@samuelbray32
Copy link
Collaborator Author

Yeah, that makes sense. Do you have a smart idea on how to delete just one of the redundant part entries? Would like to minimize cascading deletes from correcting it

@edeno
Copy link
Collaborator

edeno commented Apr 4, 2024

I don't currently. I think I tried at the time and failed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working merge To do with merge tables position
Projects
None yet
Development

No branches or pull requests

3 participants