[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9824: chore(mypy): enforcing mypy typing for connectors

[GitBox <gi...@apache.org>]

john-bodley commented on a change in pull request #9824:
URL: https://github.com/apache/incubator-superset/pull/9824#discussion_r426338096



##########
File path: superset/viz.py
##########
@@ -1055,9 +1055,9 @@ def query_obj(self):
         # dedup groupby if it happens to be the same
         d["groupby"] = list(dict.fromkeys(d["groupby"]))
 
-        self.x_metric = form_data.get("x")
-        self.y_metric = form_data.get("y")
-        self.z_metric = form_data.get("size")
+        self.x_metric = form_data["x"]

Review comment:
       I perceive these are all required otherwise `utils.get_metric_name(...)` etc. would return `None` and cause issues when trying to index the Pandas DataFrame.

##########
File path: superset/connectors/sqla/models.py
##########
@@ -841,6 +858,7 @@ def get_sqla_query(  # sqla
 
         where_clause_and = []
         having_clause_and: List = []
+        assert filter

Review comment:
       Note this could be `None` per the function signature and thus line #862 would throw an exception, however to the best of my knowledge this hasn't happened and thus `filter` is probably _never_ `None`. 

##########
File path: superset/views/base.py
##########
@@ -359,19 +361,21 @@ def yaml_export(self, items):
 
 
 class DeleteMixin:  # pylint: disable=too-few-public-methods
-    def _delete(self, primary_key):
+    def _delete(
+        self: Union[BaseView, "DeleteMixin", DruidClusterModelView], primary_key: int,
+    ) -> None:
         """
             Delete function logic, override to implement diferent logic
             deletes the record with primary_key = primary_key
 
             :param primary_key:
                 record primary key to delete
         """
-        item = self.datamodel.get(primary_key, self._base_filters)
+        item = self.datamodel.get(primary_key, self._base_filters)  # type: ignore

Review comment:
       Because FAB isn't typed and there's no stubs it seems simpler to just ignore these.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org