From 79b4f536f5921469f3b58f326a40a0ffca641106 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Thu, 14 May 2015 14:38:44 -0500 Subject: [PATCH 1/6] Debugging autopopulate. Also, replaced RelationalOperand.count with __len__. This also provides a truth test. --- datajoint/autopopulate.py | 42 ++++++++++++++------------------ datajoint/free_relation.py | 12 +++------ datajoint/relational_operand.py | 8 +++--- tests/test_relation.py | 2 +- tests/test_relational_operand.py | 2 +- 5 files changed, 29 insertions(+), 37 deletions(-) diff --git a/datajoint/autopopulate.py b/datajoint/autopopulate.py index 3223b1b47..8dce3869b 100644 --- a/datajoint/autopopulate.py +++ b/datajoint/autopopulate.py @@ -35,38 +35,32 @@ def make_tuples(self, key): def target(self): return self - def populate(self, catch_errors=False, reserve_jobs=False, restrict=None): + def populate(self, restrict=None, suppress_errors=False, error_list=None, reserve_jobs=False): """ rel.populate() will call rel.make_tuples(key) for every primary key in self.pop_rel for which there is not already a tuple in rel. """ if not isinstance(self.pop_rel, RelationalOperand): - raise DataJointError('') + raise DataJointError('Invalid pop_rel value') self.conn._cancel_transaction() - unpopulated = self.pop_rel - self.target if not unpopulated.count: logger.info('Nothing to populate', flush=True) - if catch_errors: - error_keys, errors = [], [] - for key in unpopulated.fetch(): - self.conn._start_transaction() - n = self(key).count - if n: # already populated + for key in unpopulated.project().fetch(): + self.conn._start_transaction() + if self & key: # already populated + self.conn._cancel_transaction() + else: + print('Populating:') + pprint.pprint(key) + try: + self.make_tuples(key) + except Exception as e: self.conn._cancel_transaction() + if not suppress_errors: + raise + print(e) + if error_list is not None: + error_list += (e, key) else: - print('Populating:') - pprint.pprint(key) - try: - self.make_tuples(key) - except Exception as e: - self.conn._cancel_transaction() - if not catch_errors: - raise - print(e) - errors += [e] - error_keys += [key] - else: - self.conn._commit_transaction() - if catch_errors: - return errors, error_keys \ No newline at end of file + self.conn._commit_transaction() \ No newline at end of file diff --git a/datajoint/free_relation.py b/datajoint/free_relation.py index 922f30fdd..ff27236e8 100644 --- a/datajoint/free_relation.py +++ b/datajoint/free_relation.py @@ -86,13 +86,9 @@ def _field_to_sql(field): #TODO move this into Attribute Tuple default = 'NOT NULL' # if some default specified if field.default: - # enclose value in quotes (even numeric), except special SQL values - # or values already enclosed by the user - if field.default.upper() in mysql_constants or field.default[:1] in ["'", '"']: - default = '%s DEFAULT %s' % (default, field.default) - else: - default = '%s DEFAULT "%s"' % (default, field.default) - + # enclose value in quotes except special SQL values or already enclosed + quote = field.default.upper() not in mysql_constants and field.default[0] not in '"\'' + default += ' DEFAULT ' + ('"%s"' if quote else "%s") % field.default if any((c in r'\"' for c in field.comment)): raise DataJointError('Illegal characters in attribute comment "%s"' % field.comment) @@ -336,7 +332,7 @@ def _declare(self): if not defined_name == self.ref_name: raise DataJointError('Table name {} does not match the declared' - 'name {}'.format(expected_name, defined_name)) + 'name {}'.format(self.ref_name, defined_name)) # compile the CREATE TABLE statement # TODO: support prefix diff --git a/datajoint/relational_operand.py b/datajoint/relational_operand.py index 916f7cca7..05be70301 100644 --- a/datajoint/relational_operand.py +++ b/datajoint/relational_operand.py @@ -122,12 +122,14 @@ def make_select(self, attribute_spec=None): attribute_spec = self.heading.as_sql return 'SELECT ' + attribute_spec + ' FROM ' + self.from_clause + self.where_clause - @property - def count(self): + def __len__(self): cur = self.conn.query(self.make_select('count(*)')) return cur.fetchone()[0] def __call__(self, *args, **kwargs): + """ + calling a relation is equivalent to fetching from it + """ return self.fetch(*args, **kwargs) def fetch(self, offset=0, limit=None, order_by=None, descending=False, as_dict=False): @@ -183,7 +185,7 @@ def __repr__(self): repr_string += ' '.join([template % column for column in tup]) + '\n' if self.count > limit: repr_string += '...\n' - repr_string += ' (%d tuples)\n' % self.count + repr_string += ' (%d tuples)\n' % len(self) return repr_string def __iter__(self): diff --git a/tests/test_relation.py b/tests/test_relation.py index 29c7cb8f9..830dea26e 100644 --- a/tests/test_relation.py +++ b/tests/test_relation.py @@ -73,7 +73,7 @@ def test_compound_restriction(self): tM = t & (s & "real_id = 'M'") t1 = t & "subject_id = 1" - assert_equal(tM.count, t1.count, "Results of compound request does not have same length") + assert_equal(len(tM), len(t1), "Results of compound request does not have same length") for t1_item, tM_item in zip(sorted(t1, key=lambda item: item['trial_id']), sorted(tM, key=lambda item: item['trial_id'])): diff --git a/tests/test_relational_operand.py b/tests/test_relational_operand.py index ae7dd31ca..3a25a87d0 100644 --- a/tests/test_relational_operand.py +++ b/tests/test_relational_operand.py @@ -31,7 +31,7 @@ def test_isub(self): def test_sub(self): pass - def test_count(self): + def test_len(self): pass def test_fetch(self): From 62ca790ae2fa0fe40355ebcf1dbbf52d09d876a2 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Thu, 14 May 2015 15:09:26 -0500 Subject: [PATCH 2/6] added __contains__ to RelationalOperand --- datajoint/autopopulate.py | 41 +++++++++++++++++---------------- datajoint/relational_operand.py | 16 ++++++++++--- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/datajoint/autopopulate.py b/datajoint/autopopulate.py index 8dce3869b..493e8f201 100644 --- a/datajoint/autopopulate.py +++ b/datajoint/autopopulate.py @@ -35,32 +35,33 @@ def make_tuples(self, key): def target(self): return self - def populate(self, restrict=None, suppress_errors=False, error_list=None, reserve_jobs=False): + def populate(self, restriction=None, suppress_errors=False, error_list=None, reserve_jobs=False): """ - rel.populate() will call rel.make_tuples(key) for every primary key in self.pop_rel + rel.populate() calls rel.make_tuples(key) for every primary key in self.pop_rel for which there is not already a tuple in rel. """ if not isinstance(self.pop_rel, RelationalOperand): raise DataJointError('Invalid pop_rel value') self.conn._cancel_transaction() - unpopulated = self.pop_rel - self.target - if not unpopulated.count: + unpopulated = ((self.pop_rel & restriction) - self.target).project() + if not unpopulated: logger.info('Nothing to populate', flush=True) - for key in unpopulated.project().fetch(): - self.conn._start_transaction() - if self & key: # already populated - self.conn._cancel_transaction() - else: - print('Populating:') - pprint.pprint(key) - try: - self.make_tuples(key) - except Exception as e: + else: + for key in unpopulated(): + self.conn._start_transaction() + if key in self: # already populated self.conn._cancel_transaction() - if not suppress_errors: - raise - print(e) - if error_list is not None: - error_list += (e, key) else: - self.conn._commit_transaction() \ No newline at end of file + print('Populating:') + pprint.pprint(key) + try: + self.make_tuples(key) + except Exception as e: + self.conn._cancel_transaction() + if not suppress_errors: + raise + print(e) + if error_list is not None: + error_list += (key, e) + else: + self.conn._commit_transaction() \ No newline at end of file diff --git a/datajoint/relational_operand.py b/datajoint/relational_operand.py index 05be70301..0cf22b543 100644 --- a/datajoint/relational_operand.py +++ b/datajoint/relational_operand.py @@ -88,9 +88,10 @@ def __iand__(self, restriction): """ in-place relational restriction or semijoin """ - if self._restrictions is None: - self._restrictions = [] - self._restrictions.append(restriction) + if restriction is not None: + if self._restrictions is None: + self._restrictions = [] + self._restrictions.append(restriction) return self def __and__(self, restriction): @@ -123,9 +124,18 @@ def make_select(self, attribute_spec=None): return 'SELECT ' + attribute_spec + ' FROM ' + self.from_clause + self.where_clause def __len__(self): + """ + number of tuples in the relation. This also takes care of the truth value + """ cur = self.conn.query(self.make_select('count(*)')) return cur.fetchone()[0] + def __contains__(self, item): + """ + "item in relation" is equivalient to "len(relation & item)>0" + """ + return len(self & item)>0 + def __call__(self, *args, **kwargs): """ calling a relation is equivalent to fetching from it From 2ad935bb13a35419d2dd9a4adbbeace35f91f590 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Thu, 14 May 2015 15:28:12 -0500 Subject: [PATCH 3/6] simplified autopopulate --- datajoint/autopopulate.py | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/datajoint/autopopulate.py b/datajoint/autopopulate.py index 493e8f201..34ec337eb 100644 --- a/datajoint/autopopulate.py +++ b/datajoint/autopopulate.py @@ -1,6 +1,5 @@ from .relational_operand import RelationalOperand from . import DataJointError -import pprint import abc import logging @@ -43,25 +42,22 @@ def populate(self, restriction=None, suppress_errors=False, error_list=None, res if not isinstance(self.pop_rel, RelationalOperand): raise DataJointError('Invalid pop_rel value') self.conn._cancel_transaction() - unpopulated = ((self.pop_rel & restriction) - self.target).project() - if not unpopulated: - logger.info('Nothing to populate', flush=True) - else: - for key in unpopulated(): - self.conn._start_transaction() - if key in self: # already populated + unpopulated = (self.pop_rel - self.target) & restriction + for key in unpopulated.project(): + self.conn._start_transaction() + if key in self.target: # already populated + self.conn._cancel_transaction() + else: + print('Populating', key, flush=true) + try: + self.make_tuples(key) + except Exception as e: self.conn._cancel_transaction() + if not suppress_errors: + raise + print(e) + if error_list is not None: + error_list += (key, e) else: - print('Populating:') - pprint.pprint(key) - try: - self.make_tuples(key) - except Exception as e: - self.conn._cancel_transaction() - if not suppress_errors: - raise - print(e) - if error_list is not None: - error_list += (key, e) - else: - self.conn._commit_transaction() \ No newline at end of file + self.conn._commit_transaction() + print('Done populating.') \ No newline at end of file From a39f7ddbecedd1cf9637ca725647f96e36434332 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Fri, 15 May 2015 09:04:32 -0500 Subject: [PATCH 4/6] improved autopopulate --- datajoint/autopopulate.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/datajoint/autopopulate.py b/datajoint/autopopulate.py index 34ec337eb..027dca3ab 100644 --- a/datajoint/autopopulate.py +++ b/datajoint/autopopulate.py @@ -34,11 +34,12 @@ def make_tuples(self, key): def target(self): return self - def populate(self, restriction=None, suppress_errors=False, error_list=None, reserve_jobs=False): + def populate(self, restriction=None, suppress_errors=False, reserve_jobs=False): """ rel.populate() calls rel.make_tuples(key) for every primary key in self.pop_rel for which there is not already a tuple in rel. """ + error_list = [] if suppress_errors else None if not isinstance(self.pop_rel, RelationalOperand): raise DataJointError('Invalid pop_rel value') self.conn._cancel_transaction() @@ -48,7 +49,7 @@ def populate(self, restriction=None, suppress_errors=False, error_list=None, res if key in self.target: # already populated self.conn._cancel_transaction() else: - print('Populating', key, flush=true) + print('Populating', key, flush=True) try: self.make_tuples(key) except Exception as e: @@ -56,8 +57,8 @@ def populate(self, restriction=None, suppress_errors=False, error_list=None, res if not suppress_errors: raise print(e) - if error_list is not None: - error_list += (key, e) + error_list.append((key, e)) else: self.conn._commit_transaction() - print('Done populating.') \ No newline at end of file + print('Done populating.') + return error_list \ No newline at end of file From 1c813acdea60e2a968589fc1723065c940acd7e5 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Fri, 15 May 2015 09:33:51 -0500 Subject: [PATCH 5/6] renamed make_tuples to _make_tuples --- datajoint/autopopulate.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/datajoint/autopopulate.py b/datajoint/autopopulate.py index 027dca3ab..2a0379c21 100644 --- a/datajoint/autopopulate.py +++ b/datajoint/autopopulate.py @@ -23,7 +23,7 @@ def pop_rel(self): pass @abc.abstractmethod - def make_tuples(self, key): + def _make_tuples(self, key): """ Derived classes must implement method make_tuples that fetches data from parent tables, restricting by the given key, computes dependent attributes, and inserts the new tuples into self. @@ -36,29 +36,30 @@ def target(self): def populate(self, restriction=None, suppress_errors=False, reserve_jobs=False): """ - rel.populate() calls rel.make_tuples(key) for every primary key in self.pop_rel + rel.populate() calls rel._make_tuples(key) for every primary key in self.pop_rel for which there is not already a tuple in rel. """ error_list = [] if suppress_errors else None if not isinstance(self.pop_rel, RelationalOperand): raise DataJointError('Invalid pop_rel value') - self.conn._cancel_transaction() + self.conn._cancel_transaction() # rollback previous transaction, if any unpopulated = (self.pop_rel - self.target) & restriction for key in unpopulated.project(): self.conn._start_transaction() if key in self.target: # already populated self.conn._cancel_transaction() else: - print('Populating', key, flush=True) + logger.info('Populating: ' + str(key)) try: - self.make_tuples(key) - except Exception as e: + self._make_tuples(key) + except Exception as error: self.conn._cancel_transaction() if not suppress_errors: raise - print(e) - error_list.append((key, e)) + else: + print(error) + error_list.append((key, error)) else: self.conn._commit_transaction() - print('Done populating.') + logger.info('Done populating.', flush=True) return error_list \ No newline at end of file From c5259e63f412490bb5a1f7e22f5d373fc20335d6 Mon Sep 17 00:00:00 2001 From: Dimitri Yatsenko Date: Fri, 15 May 2015 09:57:20 -0500 Subject: [PATCH 6/6] bug fix: replaced rel.count with len(rel) --- datajoint/relational_operand.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datajoint/relational_operand.py b/datajoint/relational_operand.py index 0cf22b543..396048d4d 100644 --- a/datajoint/relational_operand.py +++ b/datajoint/relational_operand.py @@ -193,7 +193,7 @@ def __repr__(self): repr_string += ' '.join(['+' + '-'*(width-2) + '+' for _ in columns]) + '\n' for tup in rel.fetch(limit=limit): repr_string += ' '.join([template % column for column in tup]) + '\n' - if self.count > limit: + if len(self) > limit: repr_string += '...\n' repr_string += ' (%d tuples)\n' % len(self) return repr_string