[GitHub] [incubator-superset] john-bodley commented on a change in pull request #9905: style(mypy): Enforcing typing for superset.utils

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

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



##########
File path: superset/utils/core.py
##########
@@ -332,28 +339,15 @@ class JSONEncodedDict(TypeDecorator):
 
     impl = TEXT
 
-    def process_bind_param(self, value, dialect):
-        if value is not None:
-            value = json.dumps(value)
+    def process_bind_param(
+        self, value: Optional[Dict[Any, Any]], dialect: str
+    ) -> Optional[str]:
+        return json.dumps(value) if value is not None else None
 
-        return value
-
-    def process_result_value(self, value, dialect):
-        if value is not None:
-            value = json.loads(value)
-        return value
-
-
-def datetime_f(dttm):

Review comment:
       This function was unused.

##########
File path: superset/utils/cache.py
##########
@@ -14,14 +14,14 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-from typing import Callable, Optional
+from typing import Any, Callable, Optional
 
 from flask import request
 
 from superset.extensions import cache_manager
 
 
-def view_cache_key(*_, **__) -> str:
+def view_cache_key(*args: Any, **kwargs: Any) -> str:  # pylint: disable=unused-argument

Review comment:
       @villebro I think the issue with using `_` etc. is it's really not clear to the reader what these arguments are, i.e., when I was looking at typing [this](https://github.com/apache/incubator-superset/blob/master/superset/views/database/views.py#L48) method is wasn't clear what `_` represented, and although the argument is currently unused, there may be value is using it in the future (it was provided for some purpose). I needed to add some debugging statements to determine the argument type.




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