Use of mutable kwargs throughout psiflow

[Andrew-S-Rosen]

psiflow/psiflow/models/base.py

Lines 28 to 34 in e011c2b

	def evaluate_dataset(
	device: str,
	ncores: int,
	load_calculator: Callable,
	inputs: List[File] = [],
	outputs: List[File] = [],
	) -> None:

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 Parsl/parsl#2925 (comment).

[svandenhaute]

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