Use of mutable kwargs throughout psiflow

[Andrew-S-Rosen]

https://github.com/molmod/psiflow/blob/e011c2b1712396d61550717fb6e30c7b75aec290/psiflow/models/base.py#L28-L34

There are many instances in Psiflow where the default arguments are mutable objects. Is this intentional? I would imagine that this would be particularly undesirable and that one should do inputs: List[file] | None = None followed by something like inputs = {} if inputs is None else inputs in the function definition. Are these kwargs intentionally mutable, out of curiosity?

[svandenhaute]

Not really, this is inspired by the examples in the Parsl docs. I see your point about this being undesirable. I seem to remember that Parsl demands these arguments to be of type list at all times. I guess we could change all defaults to None, but then take care that each time we're calling the python_app, we supply inputs and outputs as potentially-empty lists such that Parsl won't complain. Alternatively, we use a default_factory as is done in some of the dataclasses in which mutable defaults are used as well, though I don't think this is standard Python practice for functions.

[Andrew-S-Rosen]

Ah, I see. So this related to a Parsl constraint with the inputs and outputs kwargs. I'm going to raise an issue in the Parsl repo about this one because the Parsl docs don't really promote best-practices if there are constraints like this. It could make sense for Parsl to accept None and then assume it's {}.

[Andrew-S-Rosen]

@svandenhaute: here is a follow up https://github.com/Parsl/parsl/issues/2925#issuecomment-1777334808.

[svandenhaute]

Yeah, I just disabled flake8 to complain about it for now until this is sorted in Parsl.