From 2d2b72a5d1bdbc0f87fcf7d8cc543c6a2e19d3af Mon Sep 17 00:00:00 2001 From: hyi Date: Mon, 5 Feb 2024 16:47:07 -0500 Subject: [PATCH 1/6] fixed the bug detailed in issue #301 --- icees_api/features/sql.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/icees_api/features/sql.py b/icees_api/features/sql.py index 5fe6c7f..e054b38 100644 --- a/icees_api/features/sql.py +++ b/icees_api/features/sql.py @@ -547,16 +547,27 @@ def create_cohort_view(conn, table_name, cohort_features): "feature_qualifier": { "operator": value["operator"], "value": value["value"], + } if "value" in value else { + # value only has two possible keys, value key for one value, and values key for a list of values + "operator": value["operator"], + # format value["values"] list into ("value1", "value2", ...,) for SQL IN operator query + "value": '("{}")'.format('\", \"'.join(value["values"])), }, } for key, value in cohort_features.items() ] + if cohort_features: + # only add quotation marks to value if operator is = condition_str = " AND ".join( "\"{}\" {} \"{}\"".format( feature["feature_name"], feature["feature_qualifier"]["operator"], feature["feature_qualifier"]["value"], + ) if feature["feature_qualifier"]["operator"] == '=' else "\"{}\" {} {}".format( + feature["feature_name"], + feature["feature_qualifier"]["operator"], + feature["feature_qualifier"]["value"], ) for feature in cohort_features) From 232c9d2b6b3f964c36ffe2e43c8db64f581a2627 Mon Sep 17 00:00:00 2001 From: hyi Date: Tue, 6 Feb 2024 11:16:37 -0500 Subject: [PATCH 2/6] updated openapi version as suggested by dependabot --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 831b17f..8796e29 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ simplejson==3.18.1 tx-functional==0.1.2 numpy==1.24.1 statsmodels==0.12.0 -fastapi==0.88.0 +fastapi==0.109.0 uvicorn==0.20.0 reasoner-pydantic==1.2.0.4 redis==4.5.4 From 9e26b9a61fcdae745ace4af3775c805039ae8ab2 Mon Sep 17 00:00:00 2001 From: hyi Date: Tue, 6 Feb 2024 11:21:08 -0500 Subject: [PATCH 3/6] revert openapi version back since tests fail with updated version --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 8796e29..831b17f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -13,7 +13,7 @@ simplejson==3.18.1 tx-functional==0.1.2 numpy==1.24.1 statsmodels==0.12.0 -fastapi==0.109.0 +fastapi==0.88.0 uvicorn==0.20.0 reasoner-pydantic==1.2.0.4 redis==4.5.4 From 77585c6fdb3dc2ebcc2e4ef226c7bcbf8fa59f16 Mon Sep 17 00:00:00 2001 From: hyi Date: Wed, 7 Feb 2024 11:33:47 -0500 Subject: [PATCH 4/6] address code review comments --- icees_api/features/sql.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/icees_api/features/sql.py b/icees_api/features/sql.py index e054b38..d4c33a7 100644 --- a/icees_api/features/sql.py +++ b/icees_api/features/sql.py @@ -546,12 +546,9 @@ def create_cohort_view(conn, table_name, cohort_features): "feature_name": key, "feature_qualifier": { "operator": value["operator"], - "value": value["value"], - } if "value" in value else { # value only has two possible keys, value key for one value, and values key for a list of values - "operator": value["operator"], # format value["values"] list into ("value1", "value2", ...,) for SQL IN operator query - "value": '("{}")'.format('\", \"'.join(value["values"])), + "value": value.get("value") or '("{}")'.format('\", \"'.join(value["values"])), }, } for key, value in cohort_features.items() From 9905b9c0364818e979370ae9e30de803062febf7 Mon Sep 17 00:00:00 2001 From: hyi Date: Wed, 7 Feb 2024 14:22:01 -0500 Subject: [PATCH 5/6] further address code review comment --- icees_api/features/sql.py | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/icees_api/features/sql.py b/icees_api/features/sql.py index d4c33a7..7a02e6e 100644 --- a/icees_api/features/sql.py +++ b/icees_api/features/sql.py @@ -548,25 +548,23 @@ def create_cohort_view(conn, table_name, cohort_features): "operator": value["operator"], # value only has two possible keys, value key for one value, and values key for a list of values # format value["values"] list into ("value1", "value2", ...,) for SQL IN operator query - "value": value.get("value") or '("{}")'.format('\", \"'.join(value["values"])), - }, + "value": value.get("value") if value.get("value") is not None + else '("{}")'.format('\", \"'.join(value["values"])) + } } for key, value in cohort_features.items() ] if cohort_features: # only add quotation marks to value if operator is = - condition_str = " AND ".join( - "\"{}\" {} \"{}\"".format( - feature["feature_name"], - feature["feature_qualifier"]["operator"], - feature["feature_qualifier"]["value"], - ) if feature["feature_qualifier"]["operator"] == '=' else "\"{}\" {} {}".format( - feature["feature_name"], - feature["feature_qualifier"]["operator"], - feature["feature_qualifier"]["value"], - ) - for feature in cohort_features) + condition_str = " AND ".join(f'"{feature["feature_name"]}" ' + f'{feature["feature_qualifier"]["operator"]} ' + f'"{feature["feature_qualifier"]["value"]}"' + if feature["feature_qualifier"]["operator"] == '=' else + f'"{feature["feature_name"]}" ' + f'{feature["feature_qualifier"]["operator"]} ' + f'{feature["feature_qualifier"]["value"]}' + for feature in cohort_features) view_query = ( "CREATE VIEW tmp AS " From 3dc4dfe1e5e2029e83179ff066e4ff64ed8e94d2 Mon Sep 17 00:00:00 2001 From: hyi Date: Wed, 7 Feb 2024 15:22:36 -0500 Subject: [PATCH 6/6] replace f-string with simplified format version for better code readability --- icees_api/features/sql.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/icees_api/features/sql.py b/icees_api/features/sql.py index 7a02e6e..f60bdba 100644 --- a/icees_api/features/sql.py +++ b/icees_api/features/sql.py @@ -557,14 +557,12 @@ def create_cohort_view(conn, table_name, cohort_features): if cohort_features: # only add quotation marks to value if operator is = - condition_str = " AND ".join(f'"{feature["feature_name"]}" ' - f'{feature["feature_qualifier"]["operator"]} ' - f'"{feature["feature_qualifier"]["value"]}"' - if feature["feature_qualifier"]["operator"] == '=' else - f'"{feature["feature_name"]}" ' - f'{feature["feature_qualifier"]["operator"]} ' - f'{feature["feature_qualifier"]["value"]}' - for feature in cohort_features) + condition_str = " AND ".join( + ("\"{}\" {} \"{}\"" if feature["feature_qualifier"]["operator"] == '=' else "\"{}\" {} {}").format( + feature["feature_name"], + feature["feature_qualifier"]["operator"], + feature["feature_qualifier"]["value"] + ) for feature in cohort_features) view_query = ( "CREATE VIEW tmp AS "