Feat/gui mode #430
Feat/gui mode #430
Conversation
| @@ -127,6 +127,7 @@ def launch_fluent( | |||
| ip: str = None, | |||
| port: int = None, | |||
| cleanup_on_exit: bool = True, | |||
| show_gui: Optional[bool] = None, | |||
There was a problem hiding this comment.
Why this one as Optional, while many other arguments are also optional?
the other arguments have their optional state being the "None" value
There was a problem hiding this comment.
Yes, I noticed the same in fact, but this is a quite normal approach. In 3.10, you can write bool | None for Optional[bool].
There was a problem hiding this comment.
I suppose Patrick is implying that you could decorate the other ones while you are at it?
There was a problem hiding this comment.
I suppose Patrick is implying that you could decorate the other ones while you are at it?
Yeah, I know. I considered all that, but where do you stop?
There was a problem hiding this comment.
its simply that having a single one like this might cause more confusion ( i.e. "the others are not optional?" )
so maybe all or nothing, or put in note of a future task
There was a problem hiding this comment.
I think in this particular use case you should decorate all of them for the entire function. You are right though, it's hard to decide where to stop. You could not bother adding the decorator at all and come back to it.
There was a problem hiding this comment.
Yeah, I've removed it for now. Maybe the code is more obvious as it stands anyway.
There was a problem hiding this comment.
The typing is mainly for IDEs, and so it may be better to have this accurate.
There was a problem hiding this comment.
The typing is mainly for IDEs, and so it may be better to have this accurate.
In this PR, I've ended up simply aligning with what's there. I think it's good that any changes are as small as possible right now.
Co-authored-by: Dan Williams <[email protected]>
| @@ -200,7 +210,9 @@ def launch_fluent( | |||
| launch_string += f" {additional_arguments}" | |||
| launch_string += f' -sifile="{server_info_filepath}"' | |||
| launch_string += " -nm" | |||
| if not os.getenv("PYFLUENT_SHOW_SERVER_GUI"): | |||
| if (show_gui == False) or ( | |||
| show_gui == None and (os.getenv("PYFLUENT_SHOW_SERVER_GUI") != "1") | |||
There was a problem hiding this comment.
Suggested change
show_gui is None
There was a problem hiding this comment.
Thanks, have updated (also for boolean)
* Adding settings based api example, and other changes * Titles fix * Fixing convergance issue. Removing pandas. * Fixing typo * Adding exit commands, thubnail image * Style check fix
Co-authored-by: Dan Williams <[email protected]>
…nto feat/gui_mode
Allow Fluent GUI to be shown via show_gui argument to launch_fluent.
Tested interactively with PYFLUENT_SHOW_SERVER_GUI unset and set to 0 and 1: