Page MenuHomePhabricator

Make type_check_fn optional on DagsterType
AbandonedPublic

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

Details

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

bk

Diff Detail

Repository
R1 dagster
Branch
optional-type-check (branched from master)
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

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.