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 2021/08/12 04:28:54 UTC

[GitHub] [superset] villebro commented on a change in pull request #16146: chore(pylint): Bump Pylint to 2.9.6

villebro commented on a change in pull request #16146:
URL: https://github.com/apache/superset/pull/16146#discussion_r687376656



##########
File path: superset/cli.py
##########
@@ -480,7 +482,10 @@ def import_dashboards(path: str, recursive: bool, username: str) -> None:
             files.extend(path_object.rglob("*.json"))
         if username is not None:
             g.user = security_manager.find_user(username=username)
-        contents = {path.name: open(path).read() for path in files}
+            contents = {}
+            for path_ in files:
+                with open(path_) as file:
+                    contents[path_.name] = file.read()

Review comment:
       This is slightly unrelated, but as you're bumping the indentation, I wonder if we should attempt importing at all if `username is None`? And if so (=we assume there may be use cases where an anonymous user can import), shouldn't this added indentation be restored to its original place?

##########
File path: superset/examples/flights.py
##########
@@ -38,13 +38,13 @@ def load_flights(only_metadata: bool = False, force: bool = False) -> None:
         airports = pd.read_csv(airports_bytes, encoding="latin-1")
         airports = airports.set_index("IATA_CODE")
 
-        pdf["ds"] = (
+        pdf["ds"] = (  # pylint: disable=unsupported-assignment-operation
             pdf.YEAR.map(str) + "-0" + pdf.MONTH.map(str) + "-0" + pdf.DAY.map(str)
         )
         pdf.ds = pd.to_datetime(pdf.ds)
-        del pdf["YEAR"]
-        del pdf["MONTH"]
-        del pdf["DAY"]
+        del pdf["YEAR"]  # pylint: disable=unsupported-delete-operation
+        del pdf["MONTH"]  # pylint: disable=unsupported-delete-operation
+        del pdf["DAY"]  # pylint: disable=unsupported-delete-operation

Review comment:
       Should we use `drop()` here instead?

##########
File path: superset/connectors/sqla/models.py
##########
@@ -1145,7 +1143,7 @@ def get_sqla_query(  # pylint: disable=too-many-arguments,too-many-locals,too-ma
         having_clause_and = []
 
         for flt in filter:  # type: ignore
-            if not all([flt.get(s) for s in ["col", "op"]]):
+            if not all(flt.get(s) for s in ["col", "op"]):

Review comment:
       If `pylint` caught this this is a nice improvement 👍 




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

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

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