Strengthen some types #468
Conversation
| def add_arguments( | ||
| self, task: Any, args: Optional[Sequence[npt.NDArray[Any]]] | ||
| self, | ||
| task: Union[AutoTask, ManualTask], |
There was a problem hiding this comment.
You might expect this to be task: Task, but Task does not define add_input.
There was a problem hiding this comment.
Is it worth inserting a parent class in between to denote tasks that provide this interface?
There was a problem hiding this comment.
So actually it looks like AutoTask and ManualTask define their add_input method in incompatible ways:
def add_input(
self, store: Store, partition: Optional[PartSym] = None
) -> None:
vs
def add_input(
self,
arg: Union[Store, StorePartition],
proj: Optional[ProjFn] = None,
) -> None:
and the add_arguments function takes advantage of the fact that both add_input methods do the same when passing in scalar values. Therefore I actually don't think we need to bother with this.
There was a problem hiding this comment.
Well, if we wanted to clean this up, a base could define a specialized def add_store_input(store: Store) just for the overlapping portion of this API. But unless there is a bit more common API to put a protocol/base I guess it is nto worth it.
| def add_arguments( | ||
| self, task: Any, args: Optional[Sequence[npt.NDArray[Any]]] | ||
| self, | ||
| task: Union[AutoTask, ManualTask], |
There was a problem hiding this comment.
Is it worth inserting a parent class in between to denote tasks that provide this interface?
* Strengthen some types * Minor improvement * Fix test failure * Make FFTType public * Use Sequence in alphabetical_transpose
* Strengthen some types * Minor improvement * Fix test failure * Make FFTType public * Use Sequence in alphabetical_transpose
2 participants
No description provided.