datajoint / datajoint-python

Relational data pipelines for the science lab
https://datajoint.com/docs
GNU Lesser General Public License v2.1
170 stars 85 forks source link

Drop all parts when master is dropped in a master-part relationship. #374

Open dimitri-yatsenko opened 7 years ago

dimitri-yatsenko commented 7 years ago

This is a rare problem but it can occur.

If a part table does not reference its master, then dropping the master will result in part tables with no master. DataJoint does not know how to handle this situation and throws an error like this:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
~/dev/datajoint-python/datajoint/schema.py in spawn_missing_classes(self)
    124             try:
--> 125                 master_class = self.context[to_camel_case(groups['master'])]
    126             except KeyError:

KeyError: 'Response'

During handling of the above exception, another exception occurred:

DataJointError                            Traceback (most recent call last)
<ipython-input-4-fffe84c8c428> in <module>()
      1 from pipeline import experiment, reso, meso, shared
----> 2 from stimline import stimulus, tune

~/dev/stimuli/python/stimline/tune.py in <module>()
    122 
    123 @schema
--> 124 class Response(dj.Computed):
    125     definition = """  # response to directional stimulus
    126     -> Activity

~/dev/stimuli/python/stimline/tune.py in Response()
    185 
    186 
--> 187     schema.spawn_missing_classes()

~/dev/datajoint-python/datajoint/schema.py in spawn_missing_classes(self)
    125                 master_class = self.context[to_camel_case(groups['master'])]
    126             except KeyError:
--> 127                 raise DataJointError('The table %s does not follow DataJoint naming conventions' % table_name)
    128             part_class = type(class_name, (Part,), dict(definition=...))
    129             part_class._master = master_class

DataJointError: The table __response__meso_trial does not follow DataJoint naming conventions

Although it's unusual to create part tables that don't reference their masters, it is possible and this needs to be fixed.

I propose the following fix: Dropping a master table should result in dropping of all its parts regardless of whether they make a foreign key reference to the master or not.

ixcat commented 5 years ago

@dimitri-yatsenko should parts be allowed to not reference master?

Also, if so, this issue can probably be closed based on review of merge history ^^.

dpeg22 commented 2 years ago

Hi @dimitri-yatsenko I noticed that there used to be an argument for force=True when dropping a part table, that #957 deprecated. It seems like this issue is for part tables that don't reference their parent table and I was wondering how you may handle a situation where the part table is dependent on an upstream table, but the parent is not. Say you have a part table, foo.part, that does reference its parent table, foo, and is also dependent on an upstream table, upfoo, but foo is not, is there a way to drop foo.part without affecting foo or other part tables (foo.part2, foo.part3, etc…) attached to foo that aren’t dependent on upfoo? Thanks!

jverswijver commented 2 years ago

@dpeg22 We intentionally removed the force=True argument in datajoint 0.13.3, I am currently evaluating whether we should reintroduce this. @dimitri-yatsenko Do you have any input on this?

dpeg22 commented 2 years ago

If an example may help, the PosSource table here has two part tables (PosSource.DLCPos and PosSource.TrodesPos) that depend on PosSource and an upstream table (DLCPos, TrodesPos), which PosSource itself does not depend on.

dimitri-yatsenko commented 2 years ago

Let me think and answer carefully.

dpeg22 commented 1 year ago

@dimitri-yatsenko I was wondering if there is an update on this?

CBroz1 commented 10 months ago

In the current version, user_tables.Part.drop has a force arg here to circumvent an error regarding dropping a part w/o dropping the master. If set to True, the user encounters a similar error here. These errors seem to be redundant. Is the aim to prevent this drop even if force=True? The drop can be achieved with drop_quick, so perhaps force=True should result in super().drop_quick()?