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

Fault-permit insert and remove mutual exclusivity protections on Merge #824

Merged
merged 5 commits into from
Feb 10, 2024

Conversation

CBroz1
Copy link
Member

@CBroz1 CBroz1 commented Feb 9, 2024

Description

Adds fault-permit insert process and a new table to common_usage to keep track of errors across sessions. Will help monitor for recurring issues.

  • Fixes Make insert_sessions more robust to failures to populate #719: Adds try/catch to populate_all_common. For each fail, adds entry to InsertError table and returns the IDs of entries to the user.
    • common/populate_all_common.py: Turn tables into a list to loop through, avoiding repeated logger calls.
    • common/common_behav.py: Adds a populate method to handle the fact that PositionSource was the only table in the previous list without a populate method. If unwanted, this change could instead be migrated to populate_all_common which would loop through a set of tables and methods to be run for each.
    • This does not fully solve error when inserting an example session #804 because we expect the same fail state for the same file, but it does remove the halting nature of the issue
    • This does not fully solve Fault-permitting insert and print-out #212 because it does not print out successes
  • Fixes merge_insert uniqueness #768: Adds source to uuid generation for merge tables, avoiding the case where the same PK set across part-parents results in collision
    • utils/dj_merge.py: add secondary key to hashing func.
    • path/file.py: Description of the change
  • Misc:
    • Add merge delete method to further enforce the use of cautious_delete
    • More verbose warning when delete_downstream_merge yields nothing
    • Finally able to add merge_populate to avoid mess of part parent populate inserting into merge table
    • Removed unused import of linearization merge table at init due to circularity issue if only importing PositionOutput

Checklist:

  • This PR should be accompanied by a release: Unsure
  • (If release) I have updated the CITATION.cff
  • I have updated the CHANGELOG.md
  • I have added/edited docs/notebooks to reflect the changes

@CBroz1 CBroz1 marked this pull request as ready for review February 9, 2024 21:44
src/spyglass/common/populate_all_common.py Show resolved Hide resolved
src/spyglass/common/populate_all_common.py Outdated Show resolved Hide resolved
src/spyglass/common/populate_all_common.py Show resolved Hide resolved
src/spyglass/common/populate_all_common.py Outdated Show resolved Hide resolved
@CBroz1 CBroz1 mentioned this pull request Feb 10, 2024
4 tasks
@edeno edeno merged commit c0e0ee9 into LorenFrankLab:master Feb 10, 2024
7 checks passed
edeno added a commit that referenced this pull request Feb 20, 2024
* Fault-permit insert and remove mutual exclusivity protections on Merge (#824)

* #719, #804, #212

* #768

* Add merge delete and populate

* Changes following PR review @edeno

* Replace delayed import of ImportedSpikeSorting

* Update CITATION.cff (#826)

* Update CITATION.cff

* Update change log

* Update ref

* Add MUA notebook and fix numbering.

* Only apply include labels filter if include_labels not empty (#827)

* dont skip unit if include_labels list is empty

* update check for np array size

* gh-actions docs fixes (#828)

* Update 'latest' in docs deploy

* Docs bugfix. Rename Link action, incorporate cspell.

* MUA as own heading

* Update README.md

* Fix citation

* Fix citation

* include all relevant restrictions on video file

* Proposed structure for user roles. (#832)

* Add roles

* Remove use of unix user group. Add note for retroactive role assign

* Add docs on roles and external tables. Reduce key length

* Fix test for update of position tools (#835)

Related to single LED halving the data bug

* fix build error in mamba and restriction for dlc

* flush stdout before converting mp4

* Fix notebook name (#840)

* remove deprecated yaml.safe_load function

* Fix test

* replace deprecated yaml.safe_load function

* only call no_transaction_make if video key not present

---------

Co-authored-by: Chris Brozdowski <Chris.Broz@ucsf.edu>
Co-authored-by: Eric Denovellis <edeno@users.noreply.github.com>
Co-authored-by: Eric Denovellis <edeno@bu.edu>
edeno added a commit that referenced this pull request Feb 21, 2024
* allow for  multicamera epochs

* change VideoFile definition

* update video_file_num iteration

* add update methods

* change VideoFile restriction in get_video_path

* modify formatting in position_dlc_project

* allow selective video selection in pose estimation

* add NotImplementedError to DLCProject insert

* allow for video file addition after proj creation

* fix add_video_files

* fix video_file_num determination in VideoFile

* modify add_video_files method

* change call to add_video_files

* modify interval_list_name call

* Fix linting errors

* Tested DLC multicam pipeline  (#841)

* Fault-permit insert and remove mutual exclusivity protections on Merge (#824)

* #719, #804, #212

* #768

* Add merge delete and populate

* Changes following PR review @edeno

* Replace delayed import of ImportedSpikeSorting

* Update CITATION.cff (#826)

* Update CITATION.cff

* Update change log

* Update ref

* Add MUA notebook and fix numbering.

* Only apply include labels filter if include_labels not empty (#827)

* dont skip unit if include_labels list is empty

* update check for np array size

* gh-actions docs fixes (#828)

* Update 'latest' in docs deploy

* Docs bugfix. Rename Link action, incorporate cspell.

* MUA as own heading

* Update README.md

* Fix citation

* Fix citation

* include all relevant restrictions on video file

* Proposed structure for user roles. (#832)

* Add roles

* Remove use of unix user group. Add note for retroactive role assign

* Add docs on roles and external tables. Reduce key length

* Fix test for update of position tools (#835)

Related to single LED halving the data bug

* fix build error in mamba and restriction for dlc

* flush stdout before converting mp4

* Fix notebook name (#840)

* remove deprecated yaml.safe_load function

* Fix test

* replace deprecated yaml.safe_load function

* only call no_transaction_make if video key not present
---------

Co-authored-by: dpeg22 <dpeg22@gmail.com>
Co-authored-by: CBroz1 <CBrozdowski@yahoo.com>
Co-authored-by: Samuel Bray <sam.bray@ucsf.edu>
Co-authored-by: Chris Brozdowski <Chris.Broz@ucsf.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

merge_insert uniqueness Make insert_sessions more robust to failures to populate
2 participants