chore: upgrade mypy and add type guards #16227
Conversation
| return cast(str, metric) | ||
| return metric # type: ignore |
There was a problem hiding this comment.
For some reason mypy didn't pick up that Union[str, AdhocMetric] becomes str after we do is_adhoc_metric(metric) above
villebro
force-pushed
the
villebro/mypy-0.910
branch
from
f19ccdc to
66c5f5c
Compare
| hooks: | ||
| - id: mypy | ||
| additional_dependencies: [types-all] |
There was a problem hiding this comment.
This was the cleanest solution I could come up with to automatically populate stubs for mypy pre-commit hooks: pre-commit-ci/issues#69 (comment)
There was a problem hiding this comment.
@villebro this is what we did at Airbnb as well. It's a real shame that Mypy doesn't install these by default if needed.
Codecov Report
@@ Coverage Diff @@
## master #16227 +/- ##
==========================================
- Coverage 76.71% 76.49% -0.22%
==========================================
Files 996 996
Lines 53080 53080
Branches 6739 6739
==========================================
- Hits 40721 40605 -116
- Misses 12130 12246 +116
Partials 229 229
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| hooks: | ||
| - id: mypy | ||
| additional_dependencies: [types-all] |
There was a problem hiding this comment.
@villebro this is what we did at Airbnb as well. It's a real shame that Mypy doesn't install these by default if needed.
| @@ -57,7 +57,7 @@ class AdhocMetricColumn(TypedDict, total=False): | |||
| class AdhocMetric(TypedDict, total=False): | |||
| aggregate: str | |||
| column: Optional[AdhocMetricColumn] | |||
| expressionType: str | |||
| expressionType: Literal["SIMPLE", "SQL"] | |||
There was a problem hiding this comment.
I guess this is screaming out to be an enum. Definitely a future TODO.
There was a problem hiding this comment.
Good idea. In the next iteration I'll see if I can convert some of these Literal's to Enums. But it may well be that this is the most accurate representation of this type, as we want to indicate that this is a string that can only take on two values (equivalent of TypeScript's expressionType: 'SIMPLE' | 'SQL')
…gies * upstream/master: (64 commits) check roles before fetching reports (#16260) chore: upgrade mypy and add type guards (#16227) fix: pivot columns with ints for name (#16259) chore(pylint): Bump Pylint to 2.9.6 (#16146) fix examples tab for dashboard (#16253) chore: bump superset-ui packages to 0.17.84 (#16251) chore: Shows the dataset description in the gallery dropdown (#16200) fix(Dashboard): Omnibar dropdown visibility and keyboard commands (#16168) chore: bump py version for integration test (#16213) fix: skip perms on query context update (#16250) refactor: external metadata fetch API (#16193) feat(dao): admin can remove self from object owners (#15149) fix(dashboard): cross filter chart highlight when filters badge icon clicked (#16233) fix: validate_parameters and query (#16241) fix: Remove Advanced Analytics tag for 2 charts (#16240) Revert "feat: Changing Dataset names (#16199)" (#16235) feat: Allow users to connect via legacy SQLA form (#16201) fix: remove encryption from db params (#16214) fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (#16060) Show/hide tooltips (#16192) ... # Conflicts: # superset/tasks/caching/cache_strategy.py
mistercrunch
added
🏷️ bot
SUMMARY
This is a follow-up to #16194 to introduce support for
TypeGuardintroduced in Python 3.10.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION