Skip to content

Commit

Permalink
Merge delete parent fix (LorenFrankLab#617)
Browse files Browse the repository at this point in the history
* Restructure for mkdocs

* Minor docstring edits for mkdocs

* Adjust installer docs

* Permit json config

* Update docstrings for mkdocs. Add images

* Minor fixes. Rename publish action. Add changelog notes

* hard wrap changes, markdownlint

* blackify

* Fix version cmd. Minor doc wording

* mkdocstrings require empty inits to identify submodules

* 🤦‍♂️ Publish docs CI/CD `main`->`master`

* Edit `get_part`; Add `merge_fetch`

* Spelling fixes

* Refactor restrict_parts. Adjust mkdocs nav.

* Docs adjust for Merge tables

* Fix `merge_get_part`

* Use hatch for docs version

* Update changelog

* Typo

* Spellcheck config

* WIP: dict/str ristrict consistency

* Normalize restriction/classmeth. Add notes on why

* Typos

* Docs publish on tag

* Edit changelog: Add links, patch version bump

* 🧪 Test gh-actions debug

* get-part multi-source flag

* Add mutual exclusivity flag from pos branch

* See details. Notebook work, config overhaul

gitignore: add exclude example config
mkdocs: new notebook names
notebooks: complete revamp for minirec data and more links to docs
init: add new load_config, isort imports
common_lab:
	- adjust to accept names in Last, First format (nwb-compliant)
	- continue to use First Last name structure in database - yes?
common_nwb: use new load_config, change `assert` to `raise`
insert_sessions: permit paths, use file name, use raw dir
storage_dirs: remove redundant funcs for base_dir
settings: implement new base_dir system
	- allows base/raw/etc to be independent
	- defaults to dj.config, then env vars, then sets default rel paths

* WIP: fix failing tests related to base_dir edits

* underscore-prefix Merge. Linter fixes

* See Details. Notebook overhaul

- dj_config: accept base dir as arg, refactor for single responsibility
- mkdocs, installation.md
  - condense installation information to single page
  - reference new notebook
  - remove local and production subpages due to redundancy
- environment and env_position.yml: add install current dir to avoid
  additional step in installation process
- notebooks: rewrite with Docker optional and minirec as demo data
- common_lab: raise error for invalid name
- common_position: get raw dir from settings, not hardcode
- settings.py: Should this be a class with properties?
  - add options for kachery dirs set via same dj_config mechanism
  - add raw_dir helper function

* remove note to self

* WIP notebook edits

* Revise 04_LFP nb

* Fix LorenFrankLab#614

* Fix LorenFrankLab#609
  • Loading branch information
CBroz1 authored Aug 4, 2023
1 parent 290d40c commit 7d48c62
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions src/spyglass/utils/dj_merge_tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,16 @@ def _merge_repr(cls, restriction: str = True) -> dj.expression.Union:
)
]

if not parts:
print(f"No entries found for restriction: {restriction}")
return

primary_attrs = list(
dict.fromkeys( # get all columns from parts
iter_chain.from_iterable([p.heading.names for p in parts])
)
)
# primary_attrs.append(cls()._reserved_sk)

query = dj.U(*primary_attrs) * parts[0].proj( # declare query
..., # include all attributes from part 0
**{
Expand Down Expand Up @@ -445,10 +449,19 @@ def merge_delete_parent(
if dry_run:
return part_parents

with cls._safe_context():
super().delete(cls(), **kwargs)
for part_parent in part_parents:
super().delete(part_parent, **kwargs)
merge_ids = cls.merge_restrict(restriction).fetch(
RESERVED_PRIMARY_KEY, as_dict=True
)

# CB: Removed transaction protection here bc 'no' confirmation resp
# still resulted in deletes. If re-add, consider transaction=False
super().delete((cls & merge_ids), **kwargs)

if cls & merge_ids: # If 'no' on del prompt from above, skip below
return # User can still abort del below, but yes/no is unlikly

for part_parent in part_parents:
super().delete(part_parent, **kwargs) # add safemode=False?

@classmethod
def fetch_nwb(
Expand Down Expand Up @@ -541,8 +554,8 @@ def merge_get_part(

if not multi_source and len(sources) != 1:
raise ValueError(
f"Found multiple potential parts: {sources}\n\t"
+ "Try adding a restriction before invoking `get_part`.\n\t"
f"Found {len(sources)} potential parts: {sources}\n\t"
+ "\n\tTry adjusting the restriction. "
+ "Or permitting multiple sources with `multi_source=True`."
)

Expand Down Expand Up @@ -593,8 +606,8 @@ def merge_get_parent(

if not multi_source and len(part_parents) != 1:
raise ValueError(
f"Found multiple potential parents: {part_parents}\n\t"
+ "Try adding a string restriction when invoking `get_parent`."
f"Found {len(part_parents)} possible parents: {part_parents}"
+ "\n\tTry adjusting the restriction. "
+ "Or permitting multiple sources with `multi_source=True`."
)

Expand Down Expand Up @@ -664,12 +677,6 @@ def merge_populate(source: str, key=None):
# Aliased because underscore otherwise excludes from API docs.


_Merge = Merge

# Underscore as class name avoids errors when this included in a Diagram
# Aliased because underscore otherwise excludes from API docs.


def delete_downstream_merge(
table: dj.Table, restriction: str = True, dry_run=True, **kwargs
) -> list:
Expand Down

0 comments on commit 7d48c62

Please sign in to comment.