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/03/29 21:12:27 UTC
[GitHub] [incubator-superset] john-bodley opened a new pull request #9416:
[mypy] Enforcing typing for some modules
john-bodley opened a new pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416
### CATEGORY
Choose one
- [ ] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Adding `mypy` enforcement for a number of small modules (starting alphabetically):
- `superset.bin`
- `superset.commands`
- `superset.common`
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to verify the changes -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
to: @dpgaspar @villebro
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley merged pull request #9416: [mypy]
Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley merged pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399854728
##########
File path: superset/commands/exceptions.py
##########
@@ -25,25 +25,25 @@
class CommandException(SupersetException):
""" Common base class for Command exceptions. """
- def __repr__(self):
+ def __repr__(self) -> str:
if self._exception:
- return self._exception
- return self
+ return repr(self._exception)
Review comment:
This seemed like a bug, i.e., `__repr__` should return a `str`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on issue #9416: [mypy]
Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#issuecomment-606120420
@dpgaspar are you ok with these changes? Note I added a couple more modules; `superset.dao` and `superset.db_engines`, since I authored the PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r403491574
##########
File path: superset/dao/base.py
##########
@@ -112,7 +112,7 @@ def update(cls, model: Model, properties: Dict, commit: bool = True) -> Model:
return model
@classmethod
- def delete(cls, model: Model, commit=True):
+ def delete(cls, model: Model, commit: bool = True) -> Model:
Review comment:
I raised this in #9418 , I think that PR will benefit from this change.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399854850
##########
File path: superset/stats_logger.py
##########
@@ -24,26 +24,26 @@
class BaseStatsLogger:
"""Base class for logging realtime events"""
- def __init__(self, prefix="superset"):
+ def __init__(self, prefix: str = "superset") -> None:
self.prefix = prefix
- def key(self, key):
+ def key(self, key: str) -> str:
if self.prefix:
return self.prefix + key
return key
- def incr(self, key):
+ def incr(self, key: str) -> None:
"""Increment a counter"""
raise NotImplementedError()
- def decr(self, key):
+ def decr(self, key: str) -> None:
"""Decrement a counter"""
raise NotImplementedError()
- def timing(self, key, value):
+ def timing(self, key, value: float) -> None:
raise NotImplementedError()
- def gauge(self, key):
+ def gauge(self, key: str, value: int) -> None:
Review comment:
This was missing the gauge value. It's hard to tell from the statsD code whether this should be an `int`, `float` , or `both`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9416: [mypy]
Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#issuecomment-605706775
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=h1) Report
> Merging [#9416](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/ec795a4711390bcd93de51940cc173d4552a0f8d&el=desc) will **not change** coverage by `%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9416 +/- ##
=======================================
Coverage 59.01% 59.01%
=======================================
Files 378 378
Lines 12162 12162
Branches 3004 3004
=======================================
Hits 7177 7177
Misses 4805 4805
Partials 180 180
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=footer). Last update [ec795a4...c6f77fd](https://codecov.io/gh/apache/incubator-superset/pull/9416?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on issue #9416: [mypy]
Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on issue #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#issuecomment-609056583
ping @dpgaspar @villebro
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399854850
##########
File path: superset/stats_logger.py
##########
@@ -24,26 +24,26 @@
class BaseStatsLogger:
"""Base class for logging realtime events"""
- def __init__(self, prefix="superset"):
+ def __init__(self, prefix: str = "superset") -> None:
self.prefix = prefix
- def key(self, key):
+ def key(self, key: str) -> str:
if self.prefix:
return self.prefix + key
return key
- def incr(self, key):
+ def incr(self, key: str) -> None:
"""Increment a counter"""
raise NotImplementedError()
- def decr(self, key):
+ def decr(self, key: str) -> None:
"""Decrement a counter"""
raise NotImplementedError()
- def timing(self, key, value):
+ def timing(self, key, value: float) -> None:
raise NotImplementedError()
- def gauge(self, key):
+ def gauge(self, key: str, value: int) -> None:
Review comment:
This was missing the gauge value. It's hard to tell from the statsD code whether this should be an `int`, `float` , or `both`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r403491574
##########
File path: superset/dao/base.py
##########
@@ -112,7 +112,7 @@ def update(cls, model: Model, properties: Dict, commit: bool = True) -> Model:
return model
@classmethod
- def delete(cls, model: Model, commit=True):
+ def delete(cls, model: Model, commit: bool = True) -> Model:
Review comment:
I raised this in #9418 , I think that PR will benefit from this PR.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399854948
##########
File path: superset/common/tags.py
##########
@@ -14,14 +14,15 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
+from sqlalchemy import Metadata
+from sqlalchemy.engine import Engine
from sqlalchemy.exc import IntegrityError
from sqlalchemy.sql import and_, func, functions, join, literal, select
from superset.models.tags import ObjectTypes, TagTypes
-def add_types(engine, metadata):
+def add_types(engine: Engine, metadata: Metadata) -> None:
Review comment:
@dpgaspar could you verify if the `metadata` type is correct as this is FAB related.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] john-bodley commented on a change in pull
request #9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399860028
##########
File path: superset/commands/base.py
##########
@@ -23,7 +24,7 @@ class BaseCommand(ABC):
"""
@abstractmethod
- def run(self):
+ def run(self) -> Any:
Review comment:
@dpgaspar `Union[None, Model]` is equivalent to `Optional[Model]`. I've made the change.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9416: [mypy] Enforcing typing for some modules
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9416: [mypy] Enforcing typing for some modules
URL: https://github.com/apache/incubator-superset/pull/9416#discussion_r399855351
##########
File path: superset/commands/base.py
##########
@@ -23,7 +24,7 @@ class BaseCommand(ABC):
"""
@abstractmethod
- def run(self):
+ def run(self) -> Any:
Review comment:
What do you think about setting this to `Union[None, Model]`, will mypy complain?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org