-
Notifications
You must be signed in to change notification settings - Fork 91
Redesigned Dependencies, completed ERD, implemented relation U. #208
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
Changes from all commits
f8a98dd
fb0d260
4fbb9f3
8706bd6
3240966
57e46a7
4a7d6cf
956ac5f
1f364be
a264772
88a90b9
db3b534
a7d9c06
9f12677
da30a2b
c707dc3
f24af64
cbb09aa
9b3a655
edd91a1
3201653
9c4cc07
08fb275
1d565ff
fc2ed59
1612b62
9a3dc48
ec71731
454a4fd
c1e103b
50ea24b
4f44b4f
494ea16
a1bb98d
838b75e
37104b7
43ee432
e3aa42e
bcc1834
cee2f14
7b44378
b66b8ee
184d9ad
218150c
6a25bda
a396e61
e241cf9
84a7d2c
ea11a30
5374177
decbc32
f6a3d0b
54018c1
47088e1
f4b170e
a2d06b0
f310627
13814dd
8d4c107
b2bb987
9a8b7a5
946b9f0
14db2b7
262eceb
5b8e2f1
cf1ce1a
e1d70dd
0fce887
f149964
bb216f9
6be9bd8
2540730
a5141a8
6a6fc3c
6f2f2a0
075204e
3f094b9
486a5d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import logging | ||
| import datetime | ||
| import random | ||
| from pymysql import OperationalError | ||
| from .relational_operand import RelationalOperand, AndList | ||
| from . import DataJointError | ||
| from .base_relation import FreeRelation | ||
|
|
@@ -12,41 +13,40 @@ | |
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class AutoPopulate(metaclass=abc.ABCMeta): | ||
| class AutoPopulate: | ||
| """ | ||
| AutoPopulate is a mixin class that adds the method populate() to a Relation class. | ||
| Auto-populated relations must inherit from both Relation and AutoPopulate, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we changing this to matlab style? I liked the more verbose version
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I write tutorials and record presentations, I have to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I breaks all computed relation classes that have been written so far.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can either fix them or provide
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. most computed tables rely on the default
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A good portion of my computed and imported tables rely on this feature for more efficient computation. Let's not make things complicated by providing several ways of doing the same thing. Calling it
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. MATLAB does not yet have a default The |
||
| must define the property populated_from, and must define the callback method _make_tuples. | ||
| must define the property `key_source`, and must define the callback method _make_tuples. | ||
| """ | ||
| _jobs = None | ||
| _populated_from = None | ||
|
|
||
| @property | ||
| def populated_from(self): | ||
| def key_source(self): | ||
| """ | ||
| :return: the relation whose primary key values are passed, sequentially, to the | ||
| `_make_tuples` method when populate() is called.The default value is the | ||
| join of the parent relations. Users may override to change the granularity | ||
| or the scope of populate() calls. | ||
| """ | ||
| if self._populated_from is None: | ||
| self.connection.dependencies.load() | ||
| parents = [FreeRelation(self.target.connection, rel) for rel in self.target.parents] | ||
| self.connection.dependencies.load(self.full_table_name) | ||
| parents = self.target.parents(primary=True) | ||
| if not parents: | ||
| raise DataJointError('A relation must have parent relations to be able to be populated') | ||
| ret = parents.pop(0) | ||
| self._populated_from = FreeRelation(self.connection, parents.pop(0)).proj() | ||
| while parents: | ||
| ret *= parents.pop(0) | ||
| self._populated_from = ret | ||
| self._populated_from *= FreeRelation(self.connection, parents.pop(0)).proj() | ||
| return self._populated_from | ||
|
|
||
| @abc.abstractmethod | ||
| def _make_tuples(self, key): | ||
| """ | ||
| Derived classes must implement method _make_tuples that fetches data from tables that are | ||
| above them in the dependency hierarchy, restricting by the given key, computes dependent | ||
| attributes, and inserts the new tuples into self. | ||
| """ | ||
| raise NotImplementedError('Subclasses of AutoPopulate must implement the method "_make_tuples"') | ||
|
|
||
| @property | ||
| def target(self): | ||
|
|
@@ -58,10 +58,10 @@ def target(self): | |
|
|
||
| def populate(self, *restrictions, suppress_errors=False, reserve_jobs=False, order="original"): | ||
| """ | ||
| rel.populate() calls rel._make_tuples(key) for every primary key in self.populated_from | ||
| rel.populate() calls rel._make_tuples(key) for every primary key in self.key_source | ||
| for which there is not already a tuple in rel. | ||
|
|
||
| :param restrictions: a list of restrictions each restrict (rel.populated_from - target.proj()) | ||
| :param restrictions: a list of restrictions each restrict (rel.key_source - target.proj()) | ||
| :param suppress_errors: suppresses error if true | ||
| :param reserve_jobs: if true, reserves job to populate in asynchronous fashion | ||
| :param order: "original"|"reverse"|"random" - the order of execution | ||
|
|
@@ -73,10 +73,10 @@ def populate(self, *restrictions, suppress_errors=False, reserve_jobs=False, ord | |
| if order not in valid_order: | ||
| raise DataJointError('The order argument must be one of %s' % str(valid_order)) | ||
|
|
||
| todo = self.populated_from | ||
| todo = self.key_source | ||
| if not isinstance(todo, RelationalOperand): | ||
| raise DataJointError('Invalid populated_from value') | ||
| todo.restrict(AndList(restrictions)) | ||
| raise DataJointError('Invalid key_source value') | ||
| todo = todo & AndList(restrictions) | ||
|
|
||
| error_list = [] if suppress_errors else None | ||
|
|
||
|
|
@@ -104,7 +104,11 @@ def populate(self, *restrictions, suppress_errors=False, reserve_jobs=False, ord | |
| try: | ||
| self._make_tuples(dict(key)) | ||
| except Exception as error: | ||
| self.connection.cancel_transaction() | ||
| try: | ||
| self.connection.cancel_transaction() | ||
| except OperationalError: | ||
| pass | ||
|
|
||
| if reserve_jobs: | ||
| jobs.error(self.target.table_name, key, error_message=str(error)) | ||
| if not suppress_errors: | ||
|
|
@@ -118,14 +122,14 @@ def populate(self, *restrictions, suppress_errors=False, reserve_jobs=False, ord | |
| jobs.complete(self.target.table_name, key) | ||
| return error_list | ||
|
|
||
| def progress(self, restriction=None, display=True): | ||
| def progress(self, *restrictions, display=True): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not make
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. example? We do not have a concept of kw restrictions in the restrict operator.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we do every time we restrict by a dictionary.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this syntax would be useful:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's interesting but not used anywhere else yet. Yes, we could add this syntax. Then
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am hesitant to do that since
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. It looks elegant enough that we should implement it for both ScanInfo().populate(animal_id=5014, reserve_jobs=True, suppress_errors=True)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd use the keyword arguments in a different order for clarity, but it would be fine with me. I think we should either allow
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay, let use a simple |
||
| """ | ||
| report progress of populating this table | ||
| :return: remaining, total -- tuples to be populated | ||
| """ | ||
| todo = self.populated_from & restriction | ||
| todo = self.key_source & AndList(restrictions) | ||
| total = len(todo) | ||
| remaining = len(todo - self.target.project()) | ||
| remaining = len(todo - self.target.proj()) | ||
| if display: | ||
| print('%-20s' % self.__class__.__name__, flush=True, end=': ') | ||
| print('Completed %d of %d (%2.1f%%) %s' % | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be 1.0? I thought you killed of all issues for milestone 1.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just incremented the version. When we have a real 1.0 candidate, we can call it 0.9.1 or something.