-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parameterized domain types --- a mechanism for sub-workflow templates #4
Conversation
3f63a0f
to
7912185
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks promising!
""" | ||
Return a type-factory for parametrized domain types. | ||
|
||
The types return by the factory are created using typing.NewType. The returned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The types return by the factory are created using typing.NewType. The returned | |
The types returned by the factory are created using typing.NewType. The returned |
""" | ||
|
||
class Factory: | ||
_subtypes: Dict[str, type] = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So parametrized_domain_type
must only be called once per name
? That is, does using Monitor = parametrized_domain_type("Monitor", DataArray)
in two different files lead to distinct types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell this is the same for typing.NewType
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. I just wanted to make sure. This looks like the right thing to do.
|
||
def reduction_factory(tp: type) -> List[Callable]: | ||
def incident_monitor(x: Raw[tp]) -> IncidentMonitor[tp]: | ||
return IncidentMonitor[tp](x.monitor1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we could avoid repeating the return type. The injector should be able to convert the return value to the correct type. (Would likely need modifications to injector package.) But would this mess up type hinting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least for run-time, you can just omit it. I don't know if the type-checker needs it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it and neither mypy nor pyre like it when I just return the base type. So I guess we need to explicitly wrap the result.
def reduction_factory(tp: type) -> List[Callable]: | ||
def incident_monitor(x: Raw[tp]) -> IncidentMonitor[tp]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mypy
does not like this: tp
is a variable (holding a type) and cannot be used for type-checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could try
def reduction_factory(tp: type) -> List[Callable]:
T = TypeVar("T", bound=tp)
def incident_monitor(x: Raw[T]) -> IncidentMonitor[T]:
But this probably doesn't work either.
I think python/mypy#3331 might help to resolve these problems, but it has seen little progress over time. |
Closing, in favor of a slightly different approach. |
Initial attempts at making use of child-containers (child injectors) for support sub-workflows proved challenging: I ran into both technical issues (
injector
did not behave as I had assumed when working with child injectors) as well as UX ones (the client code was complex to see through, even for me who came up with it).Therefore, I moved to a more explicit and "pedestrian" approach. It should be easier to see through.
The main challenge here was to find a way of defining "generic" domain types. We want to use
NewType
to, e.g., turn ascipp.DataArray
into something specific, but would like to use a syntax such asIofQ[Sample]
to refer to theIofQ
of the "Sample" run.parametrized_domain_type
is the current result of this.