You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/06/02 18:02:49 UTC

[GitHub] [incubator-superset] serenajiang commented on a change in pull request #9943: style(mypy): Enforcing typing for superset module

serenajiang commented on a change in pull request #9943:
URL: https://github.com/apache/incubator-superset/pull/9943#discussion_r434058182



##########
File path: superset/app.py
##########
@@ -109,8 +111,8 @@ class AppContextTask(task_base):  # type: ignore
             abstract = True
 
             # Grab each call into the task and set up an app context
-            def __call__(self, *args, **kwargs):
-                with flask_app.app_context():
+            def __call__(self, *args: Any, **kwargs: Any) -> Any:
+                with flask_app.app_context():  # type: ignore

Review comment:
       This probably is for later, we should look into using the [flask stubs in typeshed](https://github.com/python/typeshed/tree/master/third_party/2and3/flask)

##########
File path: superset/viz.py
##########
@@ -2866,12 +2903,20 @@ def nest_values(self, levels, level=0, metric=None, dims=()):
             {
                 "name": i,
                 "val": levels[level][metric][dims][i],
-                "children": self.nest_values(levels, level + 1, metric, dims + (i,)),
+                "children": self.nest_values(levels, level + 1, metric, dims + [i]),
             }
             for i in levels[level][metric][dims].index
         ]
 
-    def nest_procs(self, procs, level=-1, dims=(), time=None):
+    def nest_procs(
+        self,
+        procs: Dict[int, pd.DataFrame],
+        level: int = -1,
+        dims: Optional[Tuple[str, ...]] = None,
+        time: Optional[Any] = None,

Review comment:
       ```suggestion
           time: Any = None,
   ```

##########
File path: superset/viz.py
##########
@@ -559,7 +563,7 @@ class TableViz(BaseViz):
     is_timeseries = False
     enforce_numerical_metrics = False
 
-    def should_be_timeseries(self):
+    def should_be_timeseries(self) -> Optional[bool]:

Review comment:
       In this case, if `should_be_timeseries()` is `None`, does that mean this is `False`? It could be good to cast the results to `bool` to avoid the weird sorts of results that occur with short circuiting + `.get`. Then, we can replace most of the `Optional[bool]` types with `bool`.




----------------------------------------------------------------
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