Page MenuHomePhabricator

(type-system-changes-9) PythonObjectType --> PythonObjectDagsterType. Eliminate define_python_dagster_type
ClosedPublic

Authored by schrockn on Thu, Jan 30, 10:14 PM.

Details

Summary

So this is RFC-ish. I've been struggling naming the idea of
creating a dagster type where all you want to do is an instance of
check. Here I'm proposing that we just use the class
PythonObjectDagsterType and eliminate the define_python_object_type.

This will make it clear that it much more similar to the DagsterType
and doesn't involve any sort of global registry

Depends on D1906

Test Plan

BK

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

schrockn created this revision.Thu, Jan 30, 10:14 PM
schrockn updated this revision to Diff 9168.Thu, Jan 30, 11:01 PM
schrockn retitled this revision from PythonObjectType --> PythonObjectDagsterType to (type-system-changes-9) PythonObjectType --> PythonObjectDagsterType. Eliminate define_python_dagster_type.
schrockn edited the summary of this revision. (Show Details)
schrockn edited the test plan for this revision. (Show Details)
schrockn added reviewers: max, alangenfeld.

upmessage

schrockn updated this revision to Diff 9170.Thu, Jan 30, 11:07 PM

upmessage

schrockn updated this revision to Diff 9172.Thu, Jan 30, 11:11 PM

upmessage

schrockn updated this revision to Diff 9178.Thu, Jan 30, 11:39 PM

upmessage

max added a comment.Tue, Feb 4, 11:20 PM

it's certainly good for user types to pass instanceof checks

alangenfeld accepted this revision.Thu, Feb 6, 11:52 PM

this seems reasonable to me given our current projected end state

  • DagsterType
  • PythonObjectDagsterType
  • (speculative) DagsterTypeMixin or whatever it ends up being
This revision is now accepted and ready to land.Thu, Feb 6, 11:52 PM