Prevent delete orphans #1002
Prevent delete orphans #1002
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
edeno
requested review from
samuelbray32 and
edeno
and removed request for
samuelbray32
|
Thanks Chris. I'll get into this tomorrow. First question from a quick glance though. It seems like the nightly cleanup of I know that at least I have used the |
That's a good point. I had assumed existing entries were just a byproduct of the current delete process. Here's a snippet to look at existing orphans... from spyglass.common import IntervalList, Session
all_orphans = IntervalList().nightly_cleanup(dry_run=True)
exp_sess_tbl = dj.U('nwb_file_name', 'lab_member_name')
orphans_by_experimenter = exp_sess_tbl & (all_orphans * Session * Session.Experimenter)
orphans_no_experimenter = all_orphans * Session - Session.Experimenter |
|
Is there a way to tie the interval orphan cleanup to ones specific to pipelines? |
I can cascade the delete restriction up, but, in doing so, I may capture restrictions used across pipelines. Is there something else you had in mind? |
|
I suppose we don't know how interval lists are generated necessarily unless they have the pipeline column. I was thinking that manually inserted vs. inserted automatically would be a useful distinction. |
| @@ -262,30 +260,27 @@ def _import_merge_tables(self): | |||
| ) | |||
There was a problem hiding this comment.
Because of how delete_downtream_parts finds part tables, should the Group tables PositionGroup and SortedSpikesGroup be imported here to in order to have them in the graph when found?
There was a problem hiding this comment.
I added these items and renamed the func accordingly
Co-authored-by: Samuel Bray <[email protected]>
|
The downstream portions of this work would be replaced by datajoint-python #1158 |
|
Sorry I don't know the graphs well enough to comment more specifically: When I run this on the master branch I'm able to execute successfully. from spyglass.position.v1 import TrodesPosV1
key = {"nwb_file_name":"minirec20230622_.nwb"}
(TrodesPosV1 & key).delete(key)but on this one I need to first import |
We can always stack more onto the list of |
My guess:
I think running deletes with this method is going to require large chunks of the package. I would lean towards having that done for the user by the function if it's required, even if it loads more than necessary |
|
I've added these potential blockers to the automatic import list |
|
@samuelbray32 are you satisfied with these changes? |
|
I think it works, just one more in case it helps with generalizability. In this line of if not (ft := self._get_node(table).get("ft")):
ft = FreeTable(self.connection, table)
self._set_node(table, "ft", ft)However def _get_node(self, table: Union[str, Table]):
"""Get node from graph."""
table = self._ensure_name(table)
table = self._ensure_names(table)
if not (node := self.graph.nodes.get(table)):
raise ValueError(
f"Table {table} not found in graph."
+ "\n\tPlease import this table and rerun"
)
return nodeI don't have a local database setup to test while the servers are down, but would having an argument in If not then fine to merge |
My first attempt at 'ignore outside nodes' looked something like this, where the check was in the deepest call, fetching/storing node info. I hit an issue in the |
Description
Changes for #951 ...
RestrGraphobject is now used to cascade a restriction down all at once. This removes the need forTableChainsindependently moving from a given node to each merge part individually.delete_downstream_mergehas been renamed todelete_downstream_partsto reflect expanded scopeChanges for #948 ...
RestrGraphas above proved difficult when it came to ignoring cross-pipeline intervals. A position interval should not be deleted when position data is pared with another modality being deleted.IntervalListis given its ownnightly_cleanupfunc to mirror the others, subtracting all child tables. This clears all orphans, every time, at the cost of building that table list each time. Future work could cache this list somewhere or demote this operation to running nightly, rather than at everydeletecallAdded tests
This pr adds coverage for ...
RestrGraphis_merge_tableCoverage report
Minor changes
datafolder from tests to gitignore to ignore all file types, including newh5andshfiles from DLC and export processesdelete_downstream_partsRestrGraph:load()calls undoing cascade workcamelandensure_namemethods to handle lists of tableswarnarg to clean up verbose printouts across diff uses ofcascadeandget_ftTableChainall_ftproperty, inherit insteadChecklist:
CITATION.cffaltersnippet for release notes.CHANGELOG.mdwith PR number and description.