Page MenuHomeElementl

[mypy] update check.inst to not blank out types
ClosedPublic

Authored by alangenfeld on Jul 14 2021, 7:12 PM.

Details

Summary

inst and opt_inst were set-up to -> Any which meant that they would blank out a lot of typing information as things went through it.

Idealy the type argument to check could be uesd, but I can not figure out how, so instead just:

  • for inst make sure existing type info flows through
  • overload opt_inst to try to get that set up right too
Test Plan

mypy

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

python_modules/dagster/dagster/check/__init__.py
129–205

the core changes, everything else is fixing up errors revealed by this

python_modules/dagster/dagster/core/definitions/i_solid_definition.py
203–217

open to ideas on better patterns for this

a Node can be a Solid or a Graph, so when you are plucking Nodes out that for other contextual reasons you know will be one or the other you need some way to cast

python_modules/dagster/dagster/core/instance/__init__.py
1413–1440

this is an intentional reload right @dgibson ? from D5434

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 14 2021, 7:56 PM
Harbormaster failed remote builds in B33713: Diff 41631!

I think this looks good. Some of the checks I don't have full context on but the lack of mypy complaining seems like it should be alright (until we mypy more of the system and find out something else is broken).

python_modules/dagster/dagster/core/definitions/i_solid_definition.py
203–217

I don't think it's too bad. Better than manually performing the cast every time.

python_modules/dagster/dagster/core/definitions/solid.py
334

Am a fan of the more explicit None checks

python_modules/dagster/dagster/core/execution/plan/handle.py
13

Nice ! I have definitely found this annoying in the past.

This revision is now accepted and ready to land.Jul 20 2021, 3:52 PM

rebase, deserialize_as typing