Make type_check_fn optional on DagsterType
AbandonedPublic
Actions

Authored by sandyryza on Jan 16 2021, 4:53 PM.

Details

Reviewers

schrockn
alangenfeld
max

Summary

I came across this when trying to write a guide on how to smoke test a data pipeline. The smoke test uses dagster types to enable IO managers to auto-generate input data. There's a lot going on, so I wanted to build it up incrementally, starting with the bare minimum that's required to get the example working. The type_check_fn forces a digression.

Here's the smoke test: https://dagster.phacility.com/D5794

More generally, there are a few ways that types offer value without a type_check_fn:

Documenting outputs in Dagit
dagster_type_loaders

Requiring users to provide a type_check_fn to realize these benefits makes it more difficult to incrementally adopt Dagster.

Test Plan

Diff Detail

Repository

R1 dagster

Branch

optional-type-check (branched from master)

Lint

Lint OK

Unit

No Unit Test Coverage

Event Timeline

sandyryza created this revision.Jan 16 2021, 4:53 PM

sandyryza edited the summary of this revision. (Show Details)Jan 16 2021, 4:54 PM

sandyryza edited the summary of this revision. (Show Details)Jan 16 2021, 5:02 PM

Harbormaster completed remote builds in B24515: Diff 29853.Jan 16 2021, 5:14 PM

sandyryza requested review of this revision.Jan 16 2021, 5:14 PM

sandyryza added reviewers: schrockn, alangenfeld, max.Jan 19 2021, 4:35 PM

I think I'm ok with this - don't feel that strong in either direction. Curious for others thoughts.

Here's a thought:

change the name to type_check and then allow it to be a python type or a function. This would eliminate the need for PythonObjectDagsterType

Should dispatch in an IO manager be based on the Python type of an object or the Dagster type of the output?

My minor reservation is that when there is no type_check_fn the event stream still reports that a type check passed. I think its worth checking how difficult it would be to omit the type check event

python_modules/dagster/dagster/core/types/dagster_type.py
133–169	In the case we don't have a `type_check_fn` can we not emit any `TypeCheck` events?

This revision now requires changes to proceed.Mon, Feb 15, 4:43 PM

Closing this for now while we deal with other type stuff.

Revision Contents
Changeset List

Path

Size

python_modules/

dagster/

core/

types/

dagster_type.py

8 lines

Diff 29853

View Options

python_modules/dagster/dagster/core/types/dagster_type.py

Make type_check_fn optional on DagsterTypeAbandonedPublicActions