Merge branch 'master' of https://github.com/datajoint/datajoint-python into mp

Author: dimitri-yatsenko

CHANGELOG.md

@@ -1,10 +1,15 @@
 ## Release notes
 
 ### 0.13.3 -- TBD
+* Add - Expose proxy feature for S3 external stores (#961) PR #962
 * Bugfix - Dependencies not properly loaded on populate. (#902) PR #919
 * Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
 * Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
 * Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
+* Bugfix - Fix regression issue with `DISTINCT` clause and `GROUP_BY` (#914) PR #963
+* Bugfix - Fix sql code generation to comply with sql mode `ONLY_FULL_GROUP_BY` (#916) PR #965
+* Bugfix - Fix count for left-joined `QueryExpressions` (#951) PR #966
+* Bugfix - Fix assertion error when performing a union into a join (#930) PR #967
 
 ### 0.13.2 -- May 7, 2021
 * Update `setuptools_certificate` dependency to new name `otumat`

datajoint/expression.py

@@ -44,6 +44,9 @@ class QueryExpression:
     _heading = None
     _support = None
 
+    # If the query will be using distinct
+    _distinct = False
+
     @property
     def connection(self):
         """ a dj.Connection object """
@@ -106,9 +109,8 @@ def make_sql(self, fields=None):
         Make the SQL SELECT statement.
         :param fields: used to explicitly set the select attributes
         """
-        distinct = self.heading.names == self.primary_key
         return 'SELECT {distinct}{fields} FROM {from_}{where}'.format(
-            distinct="DISTINCT " if distinct else "",
+            distinct="DISTINCT " if self._distinct else "",
             fields=self.heading.as_sql(fields or self.heading.names),
             from_=self.from_clause(), where=self.where_clause())
 
@@ -266,9 +268,11 @@ def join(self, other, semantic_check=True, left=False):
             - join_attributes)
         # need subquery if any of the join attributes are derived
         need_subquery1 = (need_subquery1 or isinstance(self, Aggregation) or
-                          any(n in self.heading.new_attributes for n in join_attributes))
+                          any(n in self.heading.new_attributes for n in join_attributes)
+                          or isinstance(self, Union))
         need_subquery2 = (need_subquery2 or isinstance(other, Aggregation) or
-                          any(n in other.heading.new_attributes for n in join_attributes))
+                          any(n in other.heading.new_attributes for n in join_attributes)
+                          or isinstance(self, Union))
         if need_subquery1:
             self = self.make_subquery()
         if need_subquery2:
@@ -440,8 +444,10 @@ def tail(self, limit=25, **fetch_kwargs):
     def __len__(self):
         """:return: number of elements in the result set e.g. ``len(q1)``."""
         return self.connection.query(
-            'SELECT count(DISTINCT {fields}) FROM {from_}{where}'.format(
-                fields=self.heading.as_sql(self.primary_key, include_aliases=False),
+            'SELECT {select_} FROM {from_}{where}'.format(
+                select_=('count(*)' if any(self._left)
+                         else 'count(DISTINCT {fields})'.format(fields=self.heading.as_sql(
+                            self.primary_key, include_aliases=False))),
                 from_=self.from_clause(),
                 where=self.where_clause())).fetchone()[0]
 
@@ -554,7 +560,7 @@ def create(cls, arg, group, keep_all_rows=False):
         if inspect.isclass(group) and issubclass(group, QueryExpression):
             group = group()   # instantiate if a class
         assert isinstance(group, QueryExpression)
-        if keep_all_rows and len(group.support) > 1:
+        if keep_all_rows and len(group.support) > 1 or group.heading.new_attributes:
             group = group.make_subquery()  # subquery if left joining a join
         join = arg.join(group, left=keep_all_rows)  # reuse the join logic
         result = cls()
@@ -718,6 +724,7 @@ def __and__(self, other):
         if not isinstance(other, QueryExpression):
             raise DataJointError('Set U can only be restricted with a QueryExpression.')
         result = copy.copy(other)
+        result._distinct = True
         result._heading = result.heading.set_primary_key(self.primary_key)
         result = result.proj()
         return result

datajoint/s3.py

@@ -3,6 +3,7 @@
 """
 from io import BytesIO
 import minio   # https://docs.minio.io/docs/python-client-api-reference
+import urllib3
 import warnings
 import uuid
 import logging
@@ -16,9 +17,24 @@ class Folder:
     """
     A Folder instance manipulates a flat folder of objects within an S3-compatible object store
     """
-    def __init__(self, endpoint, bucket, access_key, secret_key, *, secure=False, **_):
-        self.client = minio.Minio(endpoint, access_key=access_key, secret_key=secret_key,
-                                  secure=secure)
+    def __init__(self, endpoint, bucket, access_key, secret_key, *, secure=False,
+                 proxy_server=None, **_):
+        # from https://docs.min.io/docs/python-client-api-reference
+        self.client = minio.Minio(
+            endpoint,
+            access_key=access_key,
+            secret_key=secret_key,
+            secure=secure,
+            http_client=(
+                urllib3.ProxyManager(proxy_server,
+                                     timeout=urllib3.Timeout.DEFAULT_TIMEOUT,
+                                     cert_reqs="CERT_REQUIRED",
+                                     retries=urllib3.Retry(total=5,
+                                                           backoff_factor=0.2,
+                                                           status_forcelist=[500, 502, 503,
+                                                                             504]))
+                if proxy_server else None),
+            )
         self.bucket = bucket
         if not self.client.bucket_exists(bucket):
             raise errors.BucketInaccessible('Inaccessible s3 bucket %s' % bucket)

datajoint/settings.py

@@ -137,7 +137,8 @@ def get_store_spec(self, store):
         spec['subfolding'] = spec.get('subfolding', DEFAULT_SUBFOLDING)
         spec_keys = {  # REQUIRED in uppercase and allowed in lowercase
             'file': ('PROTOCOL', 'LOCATION', 'subfolding', 'stage'),
-            's3': ('PROTOCOL', 'ENDPOINT', 'BUCKET', 'ACCESS_KEY', 'SECRET_KEY', 'LOCATION', 'secure', 'subfolding', 'stage')}
+            's3': ('PROTOCOL', 'ENDPOINT', 'BUCKET', 'ACCESS_KEY', 'SECRET_KEY', 'LOCATION',
+                   'secure', 'subfolding', 'stage', 'proxy_server')}
 
         try:
             spec_keys = spec_keys[spec.get('protocol', '').lower()]

docs-parts/intro/Releases_lang1.rst

@@ -1,9 +1,14 @@
 0.13.3 -- TBD
 ----------------------
+* Add - Expose proxy feature for S3 external stores (#961) PR #962
 * Bugfix - Dependencies not properly loaded on populate. (#902) PR #919
 * Bugfix - Replace use of numpy aliases of built-in types with built-in type. (#938) PR #939
 * Bugfix - `ExternalTable.delete` should not remove row on error (#953) PR #956
 * Bugfix - Fix error handling of remove_object function in `s3.py` (#952) PR #955
+* Bugfix - Fix sql code generation to comply with sql mode ``ONLY_FULL_GROUP_BY`` (#916) PR #965
+* Bugfix - Fix count for left-joined ``QueryExpressions`` (#951) PR #966
+* Bugfix - Fix assertion error when performing a union into a join (#930) PR #967
+* Bugfix - Fix regression issue with `DISTINCT` clause and `GROUP_BY` (#914) PR #963
 
 0.13.2 -- May 7, 2021
 ----------------------

requirements.txt

@@ -10,3 +10,4 @@ minio>=7.0.0
 matplotlib
 cryptography
 otumat
+urllib3

tests/schema.py

@@ -439,3 +439,13 @@ class SessionDateA(dj.Lookup):
         ('mouse1', '2020-12-03'),
         ('mouse1', '2020-12-04')
     ]
+
+
+@schema
+class Stimulus(dj.Lookup):
+    definition = """
+    id: int
+    ---
+    contrast: int
+    brightness: int
+    """

tests/test_aggr_regressions.py

@@ -3,10 +3,11 @@
 """
 
 import itertools
-from nose.tools import assert_equal, raises
+from nose.tools import assert_equal
 import datajoint as dj
 from . import PREFIX, CONN_INFO
-
+import uuid
+from .schema_uuid import Topic, Item, top_level_namespace_id
 schema = dj.Schema(PREFIX + '_aggr_regress', connection=dj.conn(**CONN_INFO))
 
 # --------------- ISSUE 386 -------------------
@@ -103,3 +104,27 @@ def test_issue558_part2():
     d = dict(id=3, id2=5)
     assert_equal(len(X & d), len((X & d).proj(id2='3')))
 
+
+def test_left_join_len():
+    Topic().add('jeff')
+    Item.populate()
+    Topic().add('jeff2')
+    Topic().add('jeff3')
+    q = Topic.join(Item - dict(topic_id=uuid.uuid5(top_level_namespace_id, 'jeff')),
+                   left=True)
+    qf = q.fetch()
+    assert len(q) == len(qf)
+
+
+def test_union_join():
+    # https://github.com/datajoint/datajoint-python/issues/930
+    A.insert(zip([100, 200, 300, 400, 500, 600]))
+    B.insert([(100, 11), (200, 22), (300, 33), (400, 44)])
+    q1 = B & 'id < 300'
+    q2 = B & 'id > 300'
+
+    expected_data = [{'id': 0, 'id2': 5}, {'id': 1, 'id2': 6}, {'id': 2, 'id2': 7},
+                     {'id': 3, 'id2': 8}, {'id': 4, 'id2': 9}, {'id': 100, 'id2': 11},
+                     {'id': 200, 'id2': 22}, {'id': 400, 'id2': 44}]
+
+    assert ((q1 + q2) * A).fetch(as_dict=True) == expected_data

tests/test_fetch.py

@@ -6,6 +6,7 @@
 import pandas
 import warnings
 from . import schema
+from .schema import Parent, Stimulus
 import datajoint as dj
 import os
 
@@ -287,3 +288,34 @@ def test_query_caching(self):
 
         # reset cache directory state (will fail if purge was unsuccessful)
         os.rmdir(os.path.expanduser('~/dj_query_cache'))
+
+    def test_fetch_group_by(self):
+        # https://github.com/datajoint/datajoint-python/issues/914
+
+        assert Parent().fetch('KEY', order_by='name') == [{'parent_id': 1}]
+
+    def test_dj_u_distinct(self):
+        # Test developed to see if removing DISTINCT from the select statement
+        # generation breakes the dj.U universal set imlementation
+
+        # Contents to be inserted
+        contents = [
+            (1,2,3),
+            (2,2,3),
+            (3,3,2),
+            (4,5,5)
+        ]
+        Stimulus.insert(contents)
+
+        # Query the whole table
+        test_query = Stimulus()
+
+        # Use dj.U to create a list of unique contrast and brightness combinations
+        result = dj.U('contrast', 'brightness') & test_query
+        expected_result = [{'contrast': 2, 'brightness': 3},
+                           {'contrast': 3, 'brightness': 2},
+                           {'contrast': 5, 'brightness': 5}]
+
+        fetched_result = result.fetch(as_dict=True, order_by=('contrast', 'brightness'))
+        Stimulus.delete_quick()
+        assert fetched_result == expected_result

tests/test_groupby.py

@@ -0,0 +1,11 @@
+from .schema_simple import A, D
+
+
+def test_aggr_with_proj():
+    # issue #944 - only breaks with MariaDB
+    # MariaDB implements the SQL:1992 standard that prohibits fields in the select statement that are 
+    # not also in the GROUP BY statement.
+    # An improved specification in SQL:2003 allows fields that are functionally dependent on the group by 
+    # attributes to be allowed in the select. This behavior is implemented by MySQL correctly but not MariaDB yet. 
+    # See MariaDB issue: https://jira.mariadb.org/browse/MDEV-11588
+    A.aggr(D.proj(m='id_l'), ..., n='max(m) - min(m)')