chore: bump black to 19.10b0 and mypy to 0.770

[ktmud]

CATEGORY

• Build / Development Environment

SUMMARY

@graceguo-supercat noticed black==19.10b0 and black==19.3b0 is treating function trailing commas differently. This upgrades black to 19.10b0 to bring in the fix (psf/black#763).

Mypy is also updated because it's a dependency of black.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

CI Successful build.

ADDITIONAL INFORMATION

N/A

REVIEWERS

@graceguo-supercat @john-bodley

[john-bodley]

I think the issue may because of a local misalignment of black. I’m all for bumping the version of black though you’ll also need to do this in the .pre-commit-config.yaml file.

Out of interest why was the mypy version bumped as this seems unrelated to black?

[craig-rueda]

You have a failing test, but otherwise LGTM

[ktmud]

Out of interest why was the mypy version bumped as this seems unrelated to black?

I was seeing this error while pip install -r requirements.txt:

ERROR: mypy 0.670 has requirement typed-ast<1.4.0,>=1.3.1, but you'll have typed-ast 1.4.1 which is incompatible.

It seems like black has upgraded typed-ast therefore mypy also needs to be upgrade. There was actually an error in the original commit. It should have been upgrade to 0.770, not 0.700.

[willbarrett]

@ktmud it looks like this change introduced some typing issues that were surfaced by mypi.

[ktmud]

@ktmud it looks like this change introduced some typing issues that were surfaced by mypi.

Indeed. I'm working on a fix.

[john-bodley]

@ktmud let me know if you need help with the mypy work. I'm happy to create a separate PR which bumps the mypy version and resolves these differences.

[ktmud]

@ktmud let me know if you need help with the mypy work. I'm happy to create a separate PR which bumps the mypy version and resolves these differences.

Just pushed a fix. Hope this works!

[john-bodley]

I'm surprised this was necessary as it seems is_adhoc_metric(metric) can handle both a dict and str. I wonder in adding typing there, i.e.,

is_adhoc_metric(metric: Union[Dict[str, Any], str]) -> bool:

would remove the need for isinstance(metric, dict) which is unnecessary.

[ktmud]

This and L946 are for function calls later in the current context. Union[Dict, str] was passed on to adhoc_agg_configs , saved_agg_names and saved_agg_names, but could not be accepted by get_aggregations() and resolve_postagg(), which only accept either just dict or just str.

Apparently mypy was not smart enough to figure out metric type just by the outcome of is_adhoc_metric().

[john-bodley]

I dislike the get_datasource_info(...) method from a typing perspective. The datasource_type will be non-None (the method raises otherwise) and thus I think assert datasource_type is suffice from a type checking standpoint.

[ktmud]

But it doesn't hurt to give a more user-friendly error message, would it?

[ktmud]

get_datasource_info(...) only raises on datasource_id is None, but not on datasource_type.

[john-bodley]

Any reason for this change? It makes typing less strict.

[ktmud]

I'm getting tons of errors about variables from import * being overridden. Didn't find a way to bypass it and thought it wouldn't make much sense to type: ignore them one by one in a config file.

[john-bodley]

See ^^.

[codecov-io]

Codecov Report

Merging #9378 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #9378   +/-   ##
=======================================
  Coverage   59.01%   59.01%           
=======================================
  Files         382      382           
  Lines       12191    12191           
  Branches     3014     3014           
=======================================
  Hits         7194     7194           
  Misses       4815     4815           
  Partials      182      182           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e79cf3...45906e4. Read the comment docs.

[john-bodley]

I think your points are valid. Thanks for tackling this. LGTM for now and I think as we add more type enforcement we may end up refining the logic of some of these problematic methods from a typing/logic perspective.

cc: @dpgaspar @villebro for context.

[villebro]

One small nit, LGTM.

[villebro]

Suggested change

	granularity = {"type": "period"} # type: Dict[str, Union[str, float]]
	granularity: Dict[str, Union[str, float]] = {"type": "period"}