[ci] Deprecate flake8 (apache#8409)

* [ci] Deprecate flake8

* Addressing @villebro's comments

.pre-commit-config.yaml

@@ -16,11 +16,21 @@
 #
 repos:
 -   repo: https://github.com/ambv/black
-    rev: stable
+    rev: 19.3b0
     hooks:
     -   id: black
         language_version: python3
 
+-   repo: https://github.com/asottile/seed-isort-config
+    rev: v1.9.3
+    hooks:
+    -   id: seed-isort-config
+
+-   repo: https://github.com/pre-commit/mirrors-isort
+    rev: v4.3.21
+    hooks:
+    -   id: isort
+
 -   repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v2.2.3
     hooks:
@@ -30,8 +40,3 @@ repos:
     -   id: check-added-large-files
     -   id: check-yaml
     -   id: debug-statements
-
--   repo: https://gitlab.com/pycqa/flake8
-    rev: 3.7.1
-    hooks:
-    -   id: flake8

.travis.yml

@@ -75,7 +75,10 @@ jobs:
       env: TOXENV=black
     - language: python
       python: 3.6
-      env: TOXENV=flake8
+      env: TOXENV=isort
+    - language: python
+      python: 3.6
+      env: TOXENV=mypy
     - language: python
       python: 3.6
       env: TOXENV=py36-sqlite

CONTRIBUTING.md

@@ -479,6 +479,24 @@ The Python code is auto-formatted using [Black](https://github.com/python/black)
 is configured as a pre-commit hook. There are also numerous [editor integrations](https://black.readthedocs.io/en/stable/editor_integration.html).
 
 
+## Conventions
+
+### Python
+
+Parameters in the `config.py` (which are accessible via the Flask app.config dictionary) are assummed to always be defined and thus should be accessed directly via,
+
+```python
+blueprints = app.config["BLUEPRINTS"]
+```
+
+rather than,
+
+```python
+blueprints = app.config.get("BLUEPRINTS")
+```
+
+or similar as the later will cause typing issues. The former is of type `List[Callable]` whereas the later is of type `Optional[List[Callable]]`.
+
 ## Testing
 
 ### Python Testing

requirements-dev.txt

@@ -16,11 +16,9 @@
 #
 black==19.3b0
 coverage==4.5.3
-flake8-import-order==0.18.1
-flake8-mypy==17.8.0
-flake8==3.7.7
 flask-cors==3.0.7
 ipdb==0.12
+isort==4.3.21
 mypy==0.670
 nose==1.3.7
 pip-tools==3.7.0

setup.cfg

@@ -39,3 +39,15 @@ detailed-errors = 1
 with-coverage = 1
 nocapture = 1
 cover-package = superset
+
+[isort]
+combine_as_imports = true
+include_trailing_comma = true
+line_length = 88
+known_first_party = superset
+known_third_party =alembic,backoff,bleach,celery,click,colorama,contextlib2,croniter,dateutil,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_wtf,geohash,geopy,humanize,isodate,jinja2,markdown,marshmallow,msgpack,numpy,pandas,parsedatetime,pathlib2,polyline,prison,psycopg2,pyarrow,pyhive,pytz,retry,selenium,setuptools,simplejson,sphinx_rtd_theme,sqlalchemy,sqlalchemy_utils,sqlparse,werkzeug,wtforms,wtforms_json,yaml
+multi_line_output = 3
+order_by_type = false
+
+[mypy]
+ignore_missing_imports = true

superset/__init__.py

@@ -16,19 +16,20 @@
 # under the License.
 # pylint: disable=C,R,W
 """Package's main module!"""
-from copy import deepcopy
 import json
 import logging
 import os
+from copy import deepcopy
+from typing import Any, Dict
 
+import wtforms_json
 from flask import Flask, redirect
 from flask_appbuilder import AppBuilder, IndexView, SQLA
 from flask_appbuilder.baseviews import expose
 from flask_compress import Compress
 from flask_migrate import Migrate
 from flask_talisman import Talisman
 from flask_wtf.csrf import CSRFProtect
-import wtforms_json
 
 from superset import config
 from superset.connectors.connector_registry import ConnectorRegistry
@@ -45,14 +46,14 @@
     os.makedirs(config.DATA_DIR)
 
 app = Flask(__name__)
-app.config.from_object(CONFIG_MODULE)
+app.config.from_object(CONFIG_MODULE)  # type: ignore
 conf = app.config
 
 #################################################################
 # Handling manifest file logic at app start
 #################################################################
 MANIFEST_FILE = APP_DIR + "/static/assets/dist/manifest.json"
-manifest = {}
+manifest: Dict[Any, Any] = {}
 
 
 def parse_manifest_json():
@@ -103,7 +104,7 @@ def get_manifest():
 
 #################################################################
 
-for bp in conf.get("BLUEPRINTS"):
+for bp in conf["BLUEPRINTS"]:
     try:
         print("Registering blueprint: '{}'".format(bp.name))
         app.register_blueprint(bp)
@@ -129,7 +130,7 @@ def get_manifest():
 
 migrate = Migrate(app, db, directory=APP_DIR + "/migrations")
 
-app.config.get("LOGGING_CONFIGURATOR").configure_logging(app.config, app.debug)
+app.config["LOGGING_CONFIGURATOR"].configure_logging(app.config, app.debug)
 
 if app.config.get("ENABLE_CORS"):
     from flask_cors import CORS
@@ -139,7 +140,9 @@ def get_manifest():
 if app.config.get("ENABLE_PROXY_FIX"):
     from werkzeug.middleware.proxy_fix import ProxyFix
 
-    app.wsgi_app = ProxyFix(app.wsgi_app, **app.config.get("PROXY_FIX_CONFIG"))
+    app.wsgi_app = ProxyFix(  # type: ignore
+        app.wsgi_app, **app.config.get("PROXY_FIX_CONFIG")
+    )
 
 if app.config.get("ENABLE_CHUNK_ENCODING"):
 
@@ -154,16 +157,16 @@ def __call__(self, environ, start_response):
                 environ["wsgi.input_terminated"] = True
             return self.app(environ, start_response)
 
-    app.wsgi_app = ChunkedEncodingFix(app.wsgi_app)
+    app.wsgi_app = ChunkedEncodingFix(app.wsgi_app)  # type: ignore
 
-if app.config.get("UPLOAD_FOLDER"):
+if app.config["UPLOAD_FOLDER"]:
     try:
-        os.makedirs(app.config.get("UPLOAD_FOLDER"))
+        os.makedirs(app.config["UPLOAD_FOLDER"])
     except OSError:
         pass
 
-for middleware in app.config.get("ADDITIONAL_MIDDLEWARE"):
-    app.wsgi_app = middleware(app.wsgi_app)
+for middleware in app.config["ADDITIONAL_MIDDLEWARE"]:
+    app.wsgi_app = middleware(app.wsgi_app)  # type: ignore
 
 
 class MyIndexView(IndexView):
@@ -233,9 +236,9 @@ def is_feature_enabled(feature):
 if flask_app_mutator:
     flask_app_mutator(app)
 
-from superset import views  # noqa
+from superset import views  # noqa isort:skip
 
 # Registering sources
-module_datasource_map = app.config.get("DEFAULT_MODULE_DS_MAP")
-module_datasource_map.update(app.config.get("ADDITIONAL_MODULE_DS_MAP"))
+module_datasource_map = app.config["DEFAULT_MODULE_DS_MAP"]
+module_datasource_map.update(app.config["ADDITIONAL_MODULE_DS_MAP"])
 ConnectorRegistry.register_sources(module_datasource_map)

superset/cli.py

@@ -16,17 +16,17 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=C,R,W
-from datetime import datetime
 import logging
+from datetime import datetime
 from subprocess import Popen
 from sys import stdout
 
 import click
+import yaml
 from colorama import Fore, Style
 from flask import g
 from flask_appbuilder import Model
 from pathlib2 import Path
-import yaml
 
 from superset import app, appbuilder, db, examples, security_manager
 from superset.common.tags import add_favorites, add_owners, add_types

superset/common/query_context.py

@@ -15,21 +15,21 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=C,R,W
-from datetime import datetime, timedelta
 import logging
 import pickle as pkl
+from datetime import datetime, timedelta
 from typing import Any, Dict, List, Optional
 
 import numpy as np
 import pandas as pd
 
-from superset import app, cache
-from superset import db
+from superset import app, cache, db
 from superset.connectors.base.models import BaseDatasource
 from superset.connectors.connector_registry import ConnectorRegistry
 from superset.stats_logger import BaseStatsLogger
 from superset.utils import core as utils
 from superset.utils.core import DTTM_ALIAS
+
 from .query_object import QueryObject
 
 config = app.config
@@ -59,8 +59,10 @@ def __init__(
         force: bool = False,
         custom_cache_timeout: Optional[int] = None,
     ) -> None:
-        self.datasource = ConnectorRegistry.get_datasource(
-            datasource.get("type"), int(datasource.get("id")), db.session  # noqa: T400
+        self.datasource = ConnectorRegistry.get_datasource(  # type: ignore
+            datasource.get("type"),  # type: ignore
+            int(datasource.get("id")),  # type: ignore
+            db.session,
         )
         self.queries = list(map(lambda query_obj: QueryObject(**query_obj), queries))
 

superset/common/query_object.py

@@ -15,16 +15,15 @@
 # specific language governing permissions and limitations
 # under the License.
 # pylint: disable=R
-from datetime import datetime, timedelta
 import hashlib
+from datetime import datetime, timedelta
 from typing import Any, Dict, List, Optional, Union
 
 import simplejson as json
 
 from superset import app
 from superset.utils import core as utils
 
-
 # TODO: Type Metrics dictionary with TypedDict when it becomes a vanilla python type
 # https://github.com/python/mypy/issues/5288
 
@@ -39,7 +38,7 @@ class QueryObject:
     from_dttm: datetime
     to_dttm: datetime
     is_timeseries: bool
-    time_shift: timedelta
+    time_shift: Optional[timedelta]
     groupby: List[str]
     metrics: List[Union[Dict, str]]
     row_limit: int
@@ -61,7 +60,7 @@ def __init__(
         time_shift: Optional[str] = None,
         is_timeseries: bool = False,
         timeseries_limit: int = 0,
-        row_limit: int = app.config.get("ROW_LIMIT"),
+        row_limit: int = app.config["ROW_LIMIT"],
         timeseries_limit_metric: Optional[Dict] = None,
         order_desc: bool = True,
         extras: Optional[Dict] = None,
@@ -79,13 +78,15 @@ def __init__(
         )
         self.is_timeseries = is_timeseries
         self.time_range = time_range
-        self.time_shift = utils.parse_human_timedelta(time_shift)
+        self.time_shift = (
+            utils.parse_human_timedelta(time_shift) if time_shift else None
+        )
         self.groupby = groupby or []
 
         # Temporal solution for backward compatability issue
         # due the new format of non-ad-hoc metric.
         self.metrics = [
-            metric if "expressionType" in metric else metric["label"]  # noqa: T484
+            metric if "expressionType" in metric else metric["label"]  # type: ignore
             for metric in metrics
         ]
         self.row_limit = row_limit