Revise get_group_by_shank

[CBroz1]

Description

Streamlines logic of get_group_by_shank and adds missing returns #1054

Checklist:

• No. This PR should be accompanied by a release: (yes/no/unsure)
• N/a. If release, I have updated the CITATION.cff
• No. This PR makes edits to table definitions: (yes/no)
• N/a. If table edits, I have included an alter snippet for release notes.
• N/a. If this PR makes changes to position, I ran the relevant tests locally.
• I have updated the CHANGELOG.md with PR number and description.
• N/a. I have added/edited docs/notebooks to reflect the changes

[samuelbray32]

Thanks @CBroz1

Since this is already a PR on the topic, would it be alright to only run the delete calls in set_group_by_shank in these locations if there is an entry to delete? Saves time and need to import tables for tracing the restriction graph:

spyglass/src/spyglass/spikesorting/v0/spikesorting_recording.py

Line 78 in 1071910

(SortGroup & {"nwb_file_name": nwb_file_name}).delete()

spyglass/src/spyglass/spikesorting/v1/recording.py

Line 77 in 1071910

(SortGroup & {"nwb_file_name": nwb_file_name}).delete()

example replacement:

if query := SortGroup & {"nwb_file_name": nwb_file_name}:
    query.delete()

[CBroz1]

Thanks @samuelbray32 - I see what you mean. I think I would rather edit the mixin delete to return early if self & restriction is empty. Can you think of any cases where we wouldn't want that?

[samuelbray32]

Thanks @samuelbray32 - I see what you mean. I think I would rather edit the mixin delete to return early if self & restriction is empty. Can you think of any cases where we wouldn't want that?

I can't think of a scenario where I would expect a different result calling delete with an empty restricted table. Seems like a better general solution to me!