refactor: Container launcher argument refactoring

[hpohekar]

closes #2818

Updated self.<argument_name> to self.argvals['<argument_name>'].

[raph-luc]

@hpohekar This approach is not only being used for the container launcher, that was just one example:

pyfluent/src/ansys/fluent/core/launcher/container_launcher.py

Lines 156 to 158 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

It is also being used in:

pyfluent/src/ansys/fluent/core/launcher/pim_launcher.py

Lines 144 to 146 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Lines 187 to 189 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

[hpohekar]

@hpohekar This approach is not only being used for the container launcher, that was just one example:

pyfluent/src/ansys/fluent/core/launcher/container_launcher.py

Lines 156 to 158 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

It is also being used in:

pyfluent/src/ansys/fluent/core/launcher/pim_launcher.py

Lines 144 to 146 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Lines 187 to 189 in 9916a8f

	for arg_name, arg_values in argvals.items():
	setattr(self, arg_name, arg_values)
	self.argvals = argvals

@raph-luc I think use of self.<argument_name> is good for readability than self.argvals['argument_name'].

[raph-luc]

@raph-luc I think use of self.<argument_name> is good for readability than self.argvals['argument_name'].

In my understanding, the issue isn't which one is more readable on its own, the issue is that we are using both interchangeably and that makes things unnecessarily complicated (have to keep track of both when debugging issues).

I'd say pick one (either self.<argument_name> or self.argvals['argument_name']) and stick with it. I believe that self.argvals['argument_name'] is the most flexible one so we should probably stick with that one.

[hpohekar]

@raph-luc I think use of self.<argument_name> is good for readability than self.argvals['argument_name'].

In my understanding, the issue isn't which one is more readable on its own, the issue is that we are using both interchangeably and that makes things unnecessarily complicated (have to keep track of both when debugging issues).

I'd say pick one (either self.<argument_name> or self.argvals['argument_name']) and stick with it. I believe that self.argvals['argument_name'] is the most flexible one so we should probably stick with that one.

@raph-luc I think use of self.<argument_name> is good for readability than self.argvals['argument_name'].

In my understanding, the issue isn't which one is more readable on its own, the issue is that we are using both interchangeably and that makes things unnecessarily complicated (have to keep track of both when debugging issues).

I'd say pick one (either self.<argument_name> or self.argvals['argument_name']) and stick with it. I believe that self.argvals['argument_name'] is the most flexible one so we should probably stick with that one.

I'd say pick one (either self.<argument_name>orself.argvals['argument_name']) and stick with it.

Yes, therefore we had followed the existing pattern by using self.<argument_name>.

[raph-luc]

Yes, therefore we had followed the existing pattern by using self.<argument_name>.

What do you mean? In the code excerpts that I quoted above (see #2820 (comment)) we have both self.<argument_name> and self.argvals['argument_name']

[hpohekar]

Yes, therefore we had followed the existing pattern by using self.<argument_name>.

What do you mean? In the code excerpts that I quoted above (see #2820 (comment)) we have both self.<argument_name> and self.argvals['argument_name']

We don't have self.argvals['argument_name'] anywhere after settings attributes as follows -

 for arg_name, arg_values in argvals.items(): 
     setattr(self, arg_name, arg_values) 

[raph-luc]

We don't have self.argvals['argument_name'] anywhere after settings attributes as follows -

 for arg_name, arg_values in argvals.items(): 
     setattr(self, arg_name, arg_values) 

If self.argvals['argument_name'] was not being used as you say, then why were we also setting

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 189 in 9916a8f

self.argvals = argvals

?

Do you see where I am going?

We do have self.argvals['argument_name'], because self.argvals is being used for _generate_launch_string(), _build_fluent_launch_args_string(), etc., and the dictionary items inside self.argvals are accessed there. Simultaneously, we had self.<argument_name> pointing to the same variables as self.argvals['argument_name'], therefore we were indeed using both simultaneously. Does this make sense?

[hpohekar]

We don't have self.argvals['argument_name'] anywhere after settings attributes as follows -

 for arg_name, arg_values in argvals.items(): 
     setattr(self, arg_name, arg_values) 

If self.argvals['argument_name'] was not being used as you say, then why were we also setting

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 189 in 9916a8f

self.argvals = argvals

?
Do you see where I am going?

We do have self.argvals['argument_name'], because self.argvals is being used for _generate_launch_string(), _build_fluent_launch_args_string(), etc., and the dictionary items inside self.argvals are accessed there. Simultaneously, we had self.<argument_name> pointing to the same variables as self.argvals['argument_name'], therefore we were indeed using both simultaneously. Does this make sense?

If self.argvals['argument_name'] was not being used as you say, then why were we also setting self.argvals = argvals ?

We are using self.argvals in following lines right ?

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 207 in 9916a8f

self.argvals,

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 215 in 9916a8f

kwargs = _get_subprocess_kwargs_for_fluent(self.env, self.argvals)

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 259 in 9916a8f

launcher_args=self.argvals,

How can we use argvals in __call__(self) method without setting it as an attribute of self i.e self.argvals ?

[hpohekar]

We do have self.argvals['argument_name'], because self.argvals is being used for _generate_launch_string(), _build_fluent_launch_args_string(), etc., and the dictionary items inside self.argvals are accessed there. Simultaneously, we had self.<argument_name> pointing to the same variables as self.argvals['argument_name'], therefore we were indeed using both simultaneously. Does this make sense?

We do have self.argvals['argument_name'], because self.argvals is being used for _generate_launch_string(), _build_fluent_launch_args_string(), etc., and the dictionary items inside self.argvals are accessed there.

• Right, that's the exact point, it is used as self.argvals['argument_name'] inside the _generate_launch_string(), _build_fluent_launch_args_string(), _get_subprocess_kwargs_for_fluent() not inside the __call__(self) method right ?

Simultaneously, we had self.<argument_name> pointing to the same variables as self.argvals['argument_name'], therefore we were indeed using both simultaneously. Does this make sense?

Do you mean we are setting self.argvals["fluent_debug"] = True therefore we are using self.<argument_name> and self.argvals['argument_name'] simultaneously inside __call__(self) ?

pyfluent/src/ansys/fluent/core/launcher/standalone_launcher.py

Line 203 in 9916a8f

self.argvals["fluent_debug"] = True

[hpohekar]

Closed due to failing of branch name style check.

New PR - #2822