Page MenuHomeElementl

Add explanation differentiating python types and dagster types
ClosedPublic

Authored by owen on Jul 2 2021, 8:15 PM.

Details

Summary

related to - https://github.com/dagster-io/dagster/issues/4209

I think the simplest workflow involving dagster types is to never use them as annotations (especially now that we have this nice auto-conversion of type annotations to dagster types).

While enforcing this might break some people's existing code, I think putting a top-level, easily searchable warning on the Dagster Type page should help clarify the situation for people new to the Dagster Type system.

As a side note, I think the confusion around this issue lends support to the idea of prefering a list of type_check_fns (expectations?) on Ins/Outs rather than wrapping every expectation into a single DagsterType. Type annotations are super readable and ergonomic (and also encourage people to get static type checking for their code, which is nice), and it would be nice if the mental model was something more like "ok, now that I've annotated this function, python/dagster know what type this should be, so now let's add some more constraints in the Ins/Outs" (rather than "Ok I annotated my function so mypy can type check it, now let's create a dagster type so dagster can type check it and apply constraints").

Test Plan

bk

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 2 2021, 8:37 PM
Harbormaster failed remote builds in B33126: Diff 40827!
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 2 2021, 9:03 PM
Harbormaster failed remote builds in B33137: Diff 40842!
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 6 2021, 4:22 PM
Harbormaster failed remote builds in B33231: Diff 40967!
owen requested review of this revision.Jul 6 2021, 4:44 PM

This approach makes sense to me. But let's make sure this content doesn't conflict with the "recommended pattern" below https://docs.dagster.io/concepts/types#associating-dagster-types-with-python-types

In D8691#227443, @yuhan wrote:

This approach makes sense to me. But let's make sure this content doesn't conflict with the "recommended pattern" below https://docs.dagster.io/concepts/types#associating-dagster-types-with-python-types

The example here is fine / consistent (because the EvenType is being used to annotate the function, NOT EvenDagsterType). I'll highlight that in the docs to make this clearer.

yuhan added inline comments.
docs/content/concepts/types.mdx
158

๐Ÿ‘

This revision is now accepted and ready to land.Jul 6 2021, 11:12 PM