Idea: introducing generic typing in `MultiJob` for `self.children`

[SiebeLeDe]

Main Proposal

My proposal is to include generic typing in the MultiJob class to allow for correct typing behavior in self.children overwrites.

Current issue

Say I declare my own class, TetramerEDAJob, that inherets from the MultiJob and I want to specify which kind of jobs it will contain. Doing so, gives me typing information in my code editor about the available methods which prevents me from making stupid typos and working with the wrong return type (e.g., float instead of np.array containing floats (I use VS code with Python extensions enabled). In this case, I have a MultiJob that contains ADFFragmentJob (also inhereting from a MultiJob).

class TetramerEDAJob(MultiJob):
    def __init__(self, fragments: List[ChemicalSystem], complex: ChemicalSystem, settings: Settings, print_detailed_output: bool = True, **kwargs) -> None:
        super().__init__(settings=settings, **kwargs)
        self.children: List[ADFFragmentJob] = [] 

relying on

class MultiJob(Job):
    def __init__(self, children: Optional[List[Job]] = None, childrunner: Optional["JobRunner"] = None, **kwargs: Any):
        Job.__init__(self, **kwargs)
        self.children: List[Job] = [] if children is None else children

So, I have a MultiJob consisting of MultiJobs. However, Pylance raises an "override" error saying that the type (List[ADFFragmentJob]) is not invariant since it expects an instance of the Job class specifically:

"children" overrides symbol of same name in class "MultiJob"
  Variable is mutable so its type is invariant
    Override type "List[AMSJob | ADFFragmentJob]" is not the same as base type "List[Job]")

But, the idea should be that the MultiJob may contain any kind of a (user-defined) Job that inherets from Job; it should not be limited toJob specifically (which is actually an abstract base class and should never be instanced, i.e., job_instance = Job(name=...)).

To fix this problem, I suggest to introduce generic typing by replacing List[Job] by "List[J]" with J being a TypeVar (in 3.12 or earlier versions), which means that the self.children may contain any type of job as long as the job inherets from Job, that is, shares the same fingerprint as Job by implementing abstract methods.

J = TypeVar("J", bound="Job")

class MultiJob(Job, Generic[J]):
    def __init__(self, children: Optional[List[J]] = None, childrunner: Optional["JobRunner"] = None, **kwargs: Any):
        Job.__init__(self, **kwargs)
        self.children: List[J] = [] if children is None else children

Note that J is already defined on line 61 in the python file

This fixes the Pylance error. I do also realise that other methods of the MultiJob class needs to include the generic J type as well instead of Job:

def new_children(self) -> Optional[Union[List[J], Dict[str, J]]]:
     """
     return None

and similarly for other_jobs, remove_child, __iter__, and apply_to_children.

Main benefits

• No typing errors raised by Pylance when overwriting the self.children instance attribute

Uncertainties

I believe the user can also specify multiple types for the self.children instance attribute in his/her own class implementation that inherets from (a generically-typed) MultiJob. In other words, the combination of self.children: List[Union[AMSJob, ADFFragmentJob]] = [] should work as well in addition to self.children: List[ADFFragmentJob] = []. But, I do not see the point of mixing two types of jobs in self.children if you still want to get correct type information because the type checker will struggle with deciphering which type of job you want information about in case ofself.children[0].

[dormrod]

Thanks for raising this point. Your proposal and solution seems reasonable. If you would like to open a pull request to patch this please feel free to do so.

[SiebeLeDe]

Will do!

[dormrod]

Thank you for your contribution. The change is now in trunk so I am marking this as closed