Page MenuHomeElementl

Allow classes to be decorated with @experimental

Authored by jordansanders on Jun 21 2021, 4:44 PM.
Referenced Files
F2300945: D8484.id39997.diff
Sat, Jul 2, 10:06 PM
Unknown Object (File)
Wed, Jun 29, 4:56 PM
Unknown Object (File)
Tue, Jun 28, 12:46 AM
Unknown Object (File)
Mon, Jun 27, 12:23 PM
Unknown Object (File)
Mon, Jun 27, 2:17 AM
Unknown Object (File)
Fri, Jun 24, 8:16 PM
Unknown Object (File)
Tue, Jun 21, 3:44 PM
Unknown Object (File)
Mon, Jun 20, 6:34 PM



The existing @experimental decorator doesn't work particularly well when
trying to decorate a class. Previously, for vanilla class decorations,
things would mostly work:

class Foo:

foo = Foo()

would emit:

ExperimentalWarning: "Foo" is an experimental function.

That error is mostly correct except for the fact that Foo is supposed
to be an experimental class and not an experimental function. But
it's more than just a copy error - because the @experimental decorator
is returning an _inner function, Foo truly is of type function.

The most immediate consequence is that nothing can properly subclass

class Bar(Foo):

fails with:

E   TypeError: function() argument 'code' must be code, not str

It's a bit of a cryptic error but it's throwing because you can't
subclass a function.

An interesting Dagster-specific consequence is that you can't mark
anything that subclasses ConfigurableClass as @experimental:

class Configurable(ConfigurableClass):

fails with:

E       TypeError: issubclass() arg 1 must be a class

It's throwing because issubclass expects its first argument to
be a class and we're giving it a function:

One option is to take the approach that we took when introducing
@experimental_decorator - we could add a new @experimental_class and
leave it up to the author to correctly decide when to decorate with
@experimental and when to decorate with @experimental_class.

Instead, I've decided to conditionally switch on whether the callable
being decorated is a function or a class. If it's a function, we
do the same wrapping behavior we've always done. If it's a class, we
augment __init__ to also include an experimental_class_warning but
we still return a class.

Test Plan


Diff Detail

R1 dagster
jordan-experimental (branched from master)
Lint Passed
No Test Coverage

Event Timeline

Mind adding a test for a class that defines an init fn?

Include a meaningful init in tests to prove super is being called correctly

nice! I was hoping someone would add this

This revision is now accepted and ready to land.Jun 21 2021, 6:20 PM
This revision was landed with ongoing or failed builds.Jun 21 2021, 6:26 PM
This revision was automatically updated to reflect the committed changes.