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

_get_exp_summary() fails on LabMember table #885

Closed
jguides opened this issue Mar 21, 2024 · 3 comments · Fixed by #903
Closed

_get_exp_summary() fails on LabMember table #885

jguides opened this issue Mar 21, 2024 · 3 comments · Fixed by #903
Assignees
Labels
bug Something isn't working common

Comments

@jguides
Copy link
Collaborator

jguides commented Mar 21, 2024

I get an error when attempting to delete the entry in the LabMember table with my name. This entry currently lacks my username, which prevents me from deleting from other lab tables. I would like to delete the entry so I can reinsert it with my username.

However, when I run (LabMember & {"lab_member_name": "Jennifer Guidera"}).delete(), I get the error:

Error stack
`---------------------------------------------------------------------------
DataJointError                            Traceback (most recent call last)
Cell In[48], line 1
----> 1 (LabMember & {"lab_member_name": "Jennifer Guidera"}).delete()

File ~/Src/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 ~/Src/spyglass/src/spyglass/utils/dj_mixin.py:450, in SpyglassMixin.cautious_delete(self, force_permission, *args, **kwargs)
    447 start = time()
    449 if not force_permission:
--> 450     self._check_delete_permission()
    452 merge_deletes = self.delete_downstream_merge(
    453     dry_run=True,
    454     disable_warning=True,
    455     return_parts=False,
    456 )
    458 safemode = (
    459     dj.config.get("safemode", True)
    460     if kwargs.get("safemode") is None
    461     else kwargs["safemode"]
    462 )

File ~/Src/spyglass/src/spyglass/utils/dj_mixin.py:373, in SpyglassMixin._check_delete_permission(self)
    366     logger.warn(  # Permit delete if no session connection
    367         "Could not find lab team associated with "
    368         + f"{self.__class__.__name__}."
    369         + "\nBe careful not to delete others' data."
    370     )
    371     return
--> 373 sess_summary = self._get_exp_summary()
    374 experimenters = sess_summary.fetch(self._member_pk)
    375 if None in experimenters:
    376     # TODO: Check if allow delete of remainder?

File ~/Src/spyglass/src/spyglass/utils/dj_mixin.py:325, in SpyglassMixin._get_exp_summary(self)
    320 format = dj.U(self._session_pk, self._member_pk)
    321 sess_link = self._session_connection.join(
    322     self.restriction, reverse_order=True
    323 )
--> 325 exp_missing = format & (sess_link - SesExp).proj(**empty_pk)
    326 exp_present = format & (sess_link * SesExp - exp_missing).proj()
    328 return exp_missing + exp_present

File ~/mambaforge/envs/spyglass/lib/python3.9/site-packages/datajoint/expression.py:440, in QueryExpression.proj(self, *attributes, **named_attributes)
    438     pass  # all ok
    439 try:
--> 440     raise DataJointError(
    441         "Attribute `%s` already exists"
    442         % next(
    443             a
    444             for a in compute_map
    445             if a in attributes.union(rename_map).union(replicate_map)
    446         )
    447     )
    448 except StopIteration:
    449     pass  # all ok

DataJointError: Attribute `lab_member_name` already exists`
@CBroz1
Copy link
Member

CBroz1 commented Mar 22, 2024

Oops! This looks like an edge case of the cautious_delete process. In the short term, can you insert with the update flag to add your username?

@CBroz1 CBroz1 self-assigned this Mar 22, 2024
@CBroz1 CBroz1 added the common label Mar 22, 2024
@jguides
Copy link
Collaborator Author

jguides commented Mar 22, 2024

Yes, this worked. Thanks Chris! I will leave this issue open since the underlying problem still exists.

@CBroz1 CBroz1 changed the title Cannot delete entry from LabMember table _get_exp_summary() fails on LabMember table Mar 22, 2024
@CBroz1
Copy link
Member

CBroz1 commented Mar 29, 2024

This was an issue because generating the experimenter/session summary failed when the table called already had an experimenter field. This can only occur on table about personnel and not tables associated with sessions, so these will be skipped in the permission check. In my solve, I'll return Table * Session.Experimenter from _get_exp_summary in the special case where this is called on a personnel table.

CBroz1 added a commit to CBroz1/spyglass that referenced this issue Mar 29, 2024
@CBroz1 CBroz1 linked a pull request Mar 29, 2024 that will close this issue
4 tasks
@edeno edeno added the bug Something isn't working label Mar 29, 2024
@CBroz1 CBroz1 mentioned this issue Apr 2, 2024
4 tasks
edeno pushed a commit that referenced this issue Apr 19, 2024
* #892

* #885

* #879

* Partial address of #860

* Update Changelog

* Partial solve of #886 - Ask import

* Fix failing tests

* Add note on order of inheritace

* #933

* Could not replicate fill_nan error. Reverting except clause
edeno added a commit that referenced this issue Apr 25, 2024
* Add spyglass version to created analysis nwb files (#897)

* Add sg version to created analysis nwb files

* update changelog

* Change existing source script to spyglass version (#900)

* Add pynapple support (#898)

* Preliminary code

* Add retrieval of file names

* Add get_nwb_table function

* Update docstrings

* Update CHANGELOG.md

* Hot fixes for clusterless `get_ahead_behind_distance` and `get_spike_times` (#904)

* Squeeze results

* Make method and not class method

* Update CHANGELOG.md

* fix bugs in fetch_nwb (#913)

* Check for entry in merge part table prior to insert (#922)

* check for entry in merge part table prior to insert

* update changelog

* Kachery fixes (#918)

* Prioritize datajoint filepath for getting analysis file abs_path

* remove deprecated kachery tables

* update changelog

* fix lint

---------

Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>

* remove old tables from init (#925)

* Fix improper uses of strip (#929)

Strip will remove leading characters

* Update CHANGELOG.md

* Misc Issues (#903)

* #892

* #885

* #879

* Partial address of #860

* Update Changelog

* Partial solve of #886 - Ask import

* Fix failing tests

* Add note on order of inheritace

* #933

* Could not replicate fill_nan error. Reverting except clause

* Export logger (#875)

* WIP: rebase Export process

* WIP: revise doc

* ✅ : Generate working export script

* Cleanup: Expand notebook, migrate export process from graph class to export

* Revert dj_chains related edits

* Update changelog

* Revise doc

* Address review comments #875

* Remove walrus in  eval

* prevent log on preview

* Fix arg order on fetch, iterate over restr

* Add upstream analysis files during cascade. Address false positive fetch

* Avoid regen file list on revisit node

* Bump Export.Table.restr to mediumblob

* Revise Export.Table uniqueness to include export_id

* Spikesorting quality of life helpers (#910)

* add utitlity function for finding spikesorting merge ids

* add option to select v1 sorts that didn't go through artifact detection

* add option to return merge keys as dicts for future restrictions

* Add tool to get brain region and electrode info for a spikesorting merge id

* update changelog

* style cleanup

* style cleanup

* fix restriction bug for curation_id

* account for change or radiu_um argument name in spikeinterface

* only do joins with metric curastion tables if have relevant keys in the restriction

* Update tutorial to use spikesorting merge table helper functions

* fix spelling

* Add logging of AnalysisNwbfile creation time and file size (#937)

* Add logging for any func that creates AnalysisNwbfile

* Migrate create to top of respective funcs

* Use pathlib for file size. Bump creation time to top of  in spikesort

* Clear pre_create_time on create

* get/del -> pop

* Log when file accessed (#941)

* Add logging for any func that creates AnalysisNwbfile

* Fix bug on empty delete in merge table (#940)

* fix bug on empty delete in merge table

* update changelog

* fix spelling

---------

Co-authored-by: Chris Brozdowski <[email protected]>

* Remove master restriction

* Part delete takes restriction from self

---------

Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>
Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>
edeno added a commit that referenced this issue May 6, 2024
* Create class for group parts to help propagate deletes

* spelling

* update changelog

* Part delete edits (#946)

* Add spyglass version to created analysis nwb files (#897)

* Add sg version to created analysis nwb files

* update changelog

* Change existing source script to spyglass version (#900)

* Add pynapple support (#898)

* Preliminary code

* Add retrieval of file names

* Add get_nwb_table function

* Update docstrings

* Update CHANGELOG.md

* Hot fixes for clusterless `get_ahead_behind_distance` and `get_spike_times` (#904)

* Squeeze results

* Make method and not class method

* Update CHANGELOG.md

* fix bugs in fetch_nwb (#913)

* Check for entry in merge part table prior to insert (#922)

* check for entry in merge part table prior to insert

* update changelog

* Kachery fixes (#918)

* Prioritize datajoint filepath for getting analysis file abs_path

* remove deprecated kachery tables

* update changelog

* fix lint

---------

Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>

* remove old tables from init (#925)

* Fix improper uses of strip (#929)

Strip will remove leading characters

* Update CHANGELOG.md

* Misc Issues (#903)

* #892

* #885

* #879

* Partial address of #860

* Update Changelog

* Partial solve of #886 - Ask import

* Fix failing tests

* Add note on order of inheritace

* #933

* Could not replicate fill_nan error. Reverting except clause

* Export logger (#875)

* WIP: rebase Export process

* WIP: revise doc

* ✅ : Generate working export script

* Cleanup: Expand notebook, migrate export process from graph class to export

* Revert dj_chains related edits

* Update changelog

* Revise doc

* Address review comments #875

* Remove walrus in  eval

* prevent log on preview

* Fix arg order on fetch, iterate over restr

* Add upstream analysis files during cascade. Address false positive fetch

* Avoid regen file list on revisit node

* Bump Export.Table.restr to mediumblob

* Revise Export.Table uniqueness to include export_id

* Spikesorting quality of life helpers (#910)

* add utitlity function for finding spikesorting merge ids

* add option to select v1 sorts that didn't go through artifact detection

* add option to return merge keys as dicts for future restrictions

* Add tool to get brain region and electrode info for a spikesorting merge id

* update changelog

* style cleanup

* style cleanup

* fix restriction bug for curation_id

* account for change or radiu_um argument name in spikeinterface

* only do joins with metric curastion tables if have relevant keys in the restriction

* Update tutorial to use spikesorting merge table helper functions

* fix spelling

* Add logging of AnalysisNwbfile creation time and file size (#937)

* Add logging for any func that creates AnalysisNwbfile

* Migrate create to top of respective funcs

* Use pathlib for file size. Bump creation time to top of  in spikesort

* Clear pre_create_time on create

* get/del -> pop

* Log when file accessed (#941)

* Add logging for any func that creates AnalysisNwbfile

* Fix bug on empty delete in merge table (#940)

* fix bug on empty delete in merge table

* update changelog

* fix spelling

---------

Co-authored-by: Chris Brozdowski <[email protected]>

* Remove master restriction

* Part delete takes restriction from self

---------

Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>
Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>

* Fix linting

---------

Co-authored-by: Chris Brozdowski <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>
Co-authored-by: Samuel Bray <[email protected]>
Co-authored-by: Eric Denovellis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working common
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants