feat: Initial bulk of type checking by Gobot1234 · Pull Request #4761 · ansys/pyfluent

Gobot1234 · 2025-12-18T01:55:54Z

Context

Duplicate of #4500 but on the ansys remote. Looking at this code more, I realise I've ran pyupgrade on this so don't merge this till I get the rest of the ruff stuff set up. I'm also realising there are 3 issues tied up in this, apologies for that.
Fixes #4489 and fixes #4490
Helps with #4543

Change Summary

Added a bunch of initial types for the public facing library part of #4738

Gobot1234 · 2025-12-18T01:57:02Z

-        }
-        self.argvals, self.new_session = _get_argvals_and_session(argvals)
-        if self.argvals["start_timeout"] is None:
+        self.argvals, self.new_session = _get_argvals_and_session(


This was changed as sadly there's no way to type transform a typedict to have optional keys (python isn't typescript, sad)

Gobot1234 · 2025-12-18T02:03:02Z

I'll try and get to the conflicts soonish, probably more useful to get the ruff changes in first

…s/pyfluent into jhilton/typing-improvements

…ements

…ovements

Co-authored-by: Copilot <copilot@github.com>

Copilot

Pull request overview

Copilot reviewed 94 out of 97 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

src/ansys/fluent/core/launcher/launcher.py:125

create_launcher docstring says it raises ValueError for unknown launch modes, but the implementation raises DisallowedValuesError. Either update the docstring to match the actual exception type or change the raised exception to ValueError for consistency.

    Raises
    ------
    ValueError
        If an unknown Fluent launch mode is passed.
    """
    launchers = {
        LaunchMode.STANDALONE: StandaloneLauncher,
        LaunchMode.CONTAINER: DockerLauncher,
        LaunchMode.PIM: PIMLauncher,
        LaunchMode.SLURM: SlurmLauncher,
    }

    if fluent_launch_mode in launchers:
        return launchers[fluent_launch_mode](**kwargs)
    else:
        raise DisallowedValuesError(
            "launch mode",
            fluent_launch_mode,
            [f"LaunchMode.{m.name}" for m in LaunchMode],
        )

src/ansys/fluent/core/launcher/container_launcher.py:285

DockerLauncher.__call__ can return a configuration dict when dry_run is true, but the return annotation only lists session types. This makes the type hints incorrect for callers and contradicts the function body. Update the return type to include the dry-run return value type.

    def __call__(self) -> "Meshing | PureMeshing | Solver | SolverIcing | SolverAero":
        if self.argvals["dry_run"]:
            config_dict, *_ = configure_container_dict(
                self._args,
                compose_config=self._compose_config,
                **self.argvals["container_dict"],
            )
            dict_str = dict_to_str(config_dict)
            print("\nDocker container run configuration:\n")
            print("config_dict = ")
            print(dict_str)
            return config_dict

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T14:46:41Z

+        def __getitem__(
+            self, name: str
+        ) -> "SolutionVariableInfo.SolutionVariables.SolutionVariable":
+            return self._solution_variables_info[name]
+
+        def get(
+            self, name: str
+        ) -> "SolutionVariableInfo.SolutionVariables.SolutionVariable | None":
+            """Get name from solution variables"""
+            return self._solution_variables_info.get(name)


SolutionVariableInfo.SolutionVariables.__getitem__ now raises KeyError for missing names (previously returned None via .get(name, None)). This is a backwards-incompatible behavior change for users indexing into the returned object (as shown in the class docstring examples). If this change is intentional, consider keeping the old behavior or clearly documenting it and/or updating examples to use .get() where absence is expected.

Copilot · 2026-04-30T14:46:42Z

+from ansys.fluent.core.utils.context_managers import *
+from ansys.fluent.core.utils.fluent_version import *
+from ansys.fluent.core.utils.setup_for_fluent import *



Switching to many import * re-exports appears to drop several previously-public module attributes (e.g., launcher, logger, scheduler, services, utils, etc.) because star-importing symbols does not add the submodule objects themselves to ansys.fluent.core. This is likely to break existing user code and will also fail the new tests/test_public_api.py baseline which expects those names. Consider explicitly importing and re-exporting the submodules (e.g., from . import launcher as launcher) or otherwise restoring those module attributes.

Suggested change

# Restore public submodule/package attributes on ansys.fluent.core.

# Star imports re-export symbols but do not bind the submodule objects

# themselves as attributes of this package.

from . import launcher as launcher

from . import logger as logger

from . import scheduler as scheduler

from . import services as services

from . import utils as utils

Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/ansys/fluent/core/launcher/slurm_launcher.py:579

SlurmLauncher.__init__() computes _argvals via _get_argvals_and_session(argvals) before applying config.show_fluent_gui / ui_mode. This can leave _argvals['graphics_driver'] inconsistent with the final ui_mode (because _get_graphics_driver() depends on ui_mode). Apply the show_fluent_gui logic before calling _get_argvals_and_session(), or recompute graphics_driver after updating ui_mode.

        argvals = {name: kwargs.get(name) for name in SlurmLauncherArgs.__annotations__}
        self._argvals, self._new_session = _get_argvals_and_session(argvals)
        self.file_transfer_service = file_transfer_service
        if config.show_fluent_gui:
            ui_mode = UIMode.GUI
        self._argvals["ui_mode"] = UIMode(ui_mode)
        if self._argvals["start_timeout"] is None:

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/ansys/fluent/core/launcher/slurm_launcher.py:580

In SlurmLauncher.__init__, argvals is built from SlurmLauncherArgs.__annotations__, which likely omits fields inherited from LauncherArgsBase (e.g., start_timeout, cleanup_on_exit, env, etc.). Those keys are later accessed via self._argvals["start_timeout"], self._argvals["env"], etc., which can raise KeyError and/or silently drop user-provided values. Build argvals from kwargs directly (or merge in base TypedDict keys) so all launcher args are preserved, e.g., start from argvals = dict(kwargs) and then normalize as needed.

        argvals = {name: kwargs.get(name) for name in SlurmLauncherArgs.__annotations__}
        self._argvals, self._new_session = _get_argvals_and_session(argvals)
        self.file_transfer_service = file_transfer_service
        if config.show_fluent_gui:
            ui_mode = UIMode.GUI
        self._argvals["ui_mode"] = UIMode(ui_mode)
        if self._argvals["start_timeout"] is None:
            self._argvals["start_timeout"] = -1

src/ansys/fluent/core/utils/setup_for_fluent.py:33

setup_for_fluent() forwards *args into launch_fluent(), but launch_fluent() is now keyword-only (leading * in its signature). Any positional arguments passed to setup_for_fluent() will now raise TypeError. Consider updating setup_for_fluent to accept only **kwargs (or explicitly map legacy positional args to keywords) so it remains usable after the kw-only change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-05-01T11:21:22Z

+        def __getitem__(self, name: str) -> "SolutionVariableInfo.ZonesInfo.ZoneInfo":
+            return self._zones_info[name]
+
+        def get(self, name: str) -> "SolutionVariableInfo.ZonesInfo.ZoneInfo | None":
+            """Get name from zones info"""
+            return self._zones_info.get(name)


SolutionVariableInfo.ZonesInfo.__getitem__ now raises KeyError for unknown zone names (dict indexing) whereas it previously returned None. This is a behavioral/API change. If you want stricter behavior, consider documenting it and encouraging callers to use the new get() method; otherwise keep the prior None-returning behavior for backwards compatibility.

Copilot · 2026-05-01T11:21:22Z

+        for (
+            k,
+            v,
+        ) in cast(
+            list[tuple[str, Any]],
+            inspect.getmembers_static(  # pyright: ignore[reportAttributeAccessIssue]
+                self
+            ),
+        ):
+            # FIXME: This is an actual bug due to not being in 3.10 (added in 3.11)
            if isinstance(v, (_ConfigDescriptor, property)):


The new inline comment about inspect.getmembers_static being "not in 3.10 (added in 3.11)" appears inaccurate (the function exists well before 3.11). If the issue is actually a typing/typeshed/pyright stub problem, consider rewording the comment to reflect that so future readers don’t assume a Python-version compatibility issue.

Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.

+T = TypeVar("T", bound=type)
+E = TypeVar("E")
+
+
 class _ConnectionInterface:
-    def __init__(self, create_grpc_service, error_state, supports_v1):
+    def __init__(
+        self,
+        create_grpc_service: Callable[[T, E], T],
+        error_state: E,


+    def callbacks(self) -> list[list[Callable | list | dict]]:
        """Get list of callbacks along with arguments and keyword arguments."""
        return self._service_callbacks.values()


Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

src/ansys/fluent/core/services/_protocols.py:38

ServiceProtocol currently defines an __init__(channel, metadata) signature, but many concrete service classes that now inherit it have different constructor signatures (for example StreamingService.__init__(stub, metadata) and several services also take fluent_error_state). This will cause type checkers to report incompatible overrides and undermines the goal of improving typing. Consider making ServiceProtocol a marker protocol (no __init__), or defining a permissive __init__(self, *args, **kwargs) -> None, or splitting into separate protocols for the different constructor shapes.
src/ansys/fluent/core/launcher/launcher.py:125
The create_launcher() docstring says it raises ValueError for an unknown launch mode, but the implementation raises DisallowedValuesError. Please align the docstring with the actual exception type (or change the raised exception to match the documented behavior).

    Raises
    ------
    ValueError
        If an unknown Fluent launch mode is passed.
    """
    launchers = {
        LaunchMode.STANDALONE: StandaloneLauncher,
        LaunchMode.CONTAINER: DockerLauncher,
        LaunchMode.PIM: PIMLauncher,
        LaunchMode.SLURM: SlurmLauncher,
    }

    if fluent_launch_mode in launchers:
        return launchers[fluent_launch_mode](**kwargs)
    else:
        raise DisallowedValuesError(
            "launch mode",
            fluent_launch_mode,
            [f"LaunchMode.{m.name}" for m in LaunchMode],
        )

src/ansys/fluent/core/utils/setup_for_fluent.py:34

launch_fluent() is now keyword-only (leading *), but setup_for_fluent() still accepts *args and forwards them directly. Any positional arguments passed to setup_for_fluent() will now raise TypeError at runtime. Consider making setup_for_fluent keyword-only as well (or validating that args is empty and raising a clearer error).

+        launcher = StandaloneLauncher(
+            **kwargs, dry_run=dry_run, mode=cls._session_mode[cls.__name__]
+        )


+        launcher = DockerLauncher(
+            **kwargs, dry_run=dry_run, mode=cls._session_mode[cls.__name__]
+        )


+class LaunchFluentArgs(LaunchFluentArgsNoContainer, TypedDict, total=False):
+    """Arguments for launch_fluent()."""
+
+    start_container: Literal[False] | None
+    """Specifies whether to launch a Fluent Docker container image. For more details about containers, see
+    :mod:`~ansys.fluent.core.launcher.fluent_container`.
+    """
+    container_dict: None
+    """Dictionary for Fluent Docker container configuration. If specified,
+    setting ``start_container = True`` as well is redundant.
+    Will launch Fluent inside a Docker container using the configuration changes specified.
+    See also :mod:`~ansys.fluent.core.launcher.fluent_container`.
+    """


Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

src/ansys/fluent/core/services/_protocols.py:38

ServiceProtocol defines an __init__(channel, metadata) signature (under TYPE_CHECKING). Many service classes in this PR inherit from ServiceProtocol but their constructors require additional parameters (for example fluent_error_state, stub, etc.), which will make type checking fail because the subclass __init__ is incompatible with the protocol. Consider either (a) removing __init__ from the protocol (use it as a marker protocol), or (b) widening it to accept *args/**kwargs (or an optional fluent_error_state) so implementations remain compatible.
src/ansys/fluent/core/services/solution_variables.py:229
ZonesInfo.__getitem__ changed from dict.get(..., None) to direct indexing, which will now raise KeyError for unknown zone names. Because this class appears to be part of the user-facing API (and examples show indexing usage), this is a breaking behavior change. Consider preserving the previous None-returning behavior for __getitem__, or documenting the change and encouraging callers to use get() when the key may be absent.

+T = TypeVar("T", bound=type)
+E = TypeVar("E")
+
+
 class _ConnectionInterface:
-    def __init__(self, create_grpc_service, error_state, supports_v1):
+    def __init__(
+        self,
+        create_grpc_service: Callable[[T, E], T],
+        error_state: E,
+        supports_v1: bool,
+    ):


+reportUnusedCallResult = false
+reportUnannotatedClassAttribute = false
+ignore = ["doc", "src/ansys/fluent/core/generated"]
+baselineFile = ".ci/basedpyright.json"


doubt it though

Copilot

Pull request overview

Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/ansys/fluent/core/services/_protocols.py:38

ServiceProtocol currently constrains service constructors to __init__(channel, metadata) only. Many services (and FluentConnection.create_grpc_service, which forwards *args) require additional positional parameters (for example fluent_error_state). Consider widening the protocol initializer to accept *args, **kwargs (or adding a second protocol for services that take fluent_error_state) so the protocol matches actual construction sites and doesn’t force type-checker ignores.

 from ansys.fluent.core.launcher.standalone_launcher import StandaloneLauncher
 import ansys.fluent.core.launcher.watchdog as watchdog
 from ansys.fluent.core.module_config import config
+from ansys.fluent.core.session import BaseSession


    rep = Report(ansys_libs=dependencies, ansys_vars=ANSYS_ENV_VARS)
    assert "PyAnsys Software and Environment Report" in str(rep)
-    assert str(rep).count("pandas") == 1
+    assert str(rep).count("pandas") == 2



Gobot1234 and others added 10 commits November 5, 2025 10:21

Add type level support for passing Path instances to download_file

4f2c990

Add typing_extensions

1ddf8e8

Fix inheritance for Meshing

7825ff4

Fix create_launcher allowing None

90d69a0

Update launchers typing to be more specific

654deb4

Fix ci

ec258c3

Fix ci again

00c3a85

remove accidental commits

b6d2e3e

run pyupgrade for moving typing to builtins and a couple of other things

0a48f90

fix some np.arrays that are meant to be ndarrays in types

7b36a1c

Gobot1234 commented Dec 18, 2025

View reviewed changes

mkundu1 mentioned this pull request Dec 30, 2025

100% type hints in PyFluent code #4738

Open

Gobot1234 added 5 commits January 5, 2026 16:50

pre-fix the conflict that will happen

c04b413

add types for services

08e5525

Merge branch 'jhilton/typing-improvements' of https://github.com/ansy…

7c2e9c3

…s/pyfluent into jhilton/typing-improvements

fix some low hanging fruit

0acf4ea

Merge remote-tracking branch 'origin/main' into jhilton/typing-improv…

e6a7bf9

…ements

github-actions Bot added documentation Documentation related (improving, adding, etc) examples Publishing PyFluent examples maintenance General maintenance of the repo (libraries, cicd, etc) dependencies Related to dependencies labels Jan 15, 2026

Gobot1234 commented Jan 15, 2026

View reviewed changes

Comment thread src/ansys/fluent/core/launcher/slurm_launcher.py Outdated

Gobot1234 marked this pull request as ready for review January 15, 2026 03:19

Gobot1234 requested review from hpohekar, mkundu1, prmukherj and seanpearsonuk as code owners January 15, 2026 03:19

Copilot AI review requested due to automatic review settings January 15, 2026 03:19

Gobot1234 requested a review from mayankansys as a code owner January 15, 2026 03:19

Copilot AI requested a review from mkundu1 April 29, 2026 19:58

mkundu1 mentioned this pull request Apr 29, 2026

Address copilot suggestions on the initial type-checking PR #5095

Open

Gobot1234 and others added 2 commits April 30, 2026 10:34

Merge remote-tracking branch 'upstream/main' into jhilton/typing-impr…

97d2515

…ovements

some changes from main

b39fa44

Co-authored-by: Copilot <copilot@github.com>

Copilot AI review requested due to automatic review settings April 30, 2026 14:40

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Gobot1234 added 4 commits May 1, 2026 06:29

rm ci related changes

9cfb24c

fix type hints for dry run docker launches

4579b09

address copilot comments

020e391

comment out more of ci

69bc31c

Copilot AI reviewed May 1, 2026

View reviewed changes

Gobot1234 and others added 4 commits May 1, 2026 07:01

fix nnodes return type

2f51f22

fix nameerror

0c869f3

fix return type of DockerLauncher.__call__

df835f7

Apply suggestions from code review

793efd6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI reviewed May 1, 2026

View reviewed changes

Gobot1234 and others added 3 commits May 1, 2026 08:44

add probably missing .get call

370e0b8

Merge branch 'main' into jhilton/typing-improvements

9b8a6d2

delete magic mystery file

2b9159f

Copilot AI reviewed May 12, 2026

View reviewed changes

more v1 batch ops changes

7374ef0

Copilot AI reviewed May 12, 2026

View reviewed changes

Gobot1234 added 2 commits May 13, 2026 05:40

fix typo

d28286c

should actually work now

0ab9025

Copilot AI reviewed May 13, 2026

View reviewed changes

Gobot1234 and others added 2 commits May 21, 2026 02:38

maybe claude has actually one shot this

491bc36

doubt it though

Merge branch 'main' into jhilton/typing-improvements

1cfcedf

Copilot AI reviewed May 21, 2026

View reviewed changes

Gobot1234 and others added 2 commits May 21, 2026 10:20

fix fmt

e9864cd

chore: adding changelog file 4761.added.md [dependabot-skip]

8a2c123

+# Restore public submodule/package attributes on ansys.fluent.core.
+# Star imports re-export symbols but do not bind the submodule objects
+# themselves as attributes of this package.
+from . import launcher as launcher
+from . import logger as logger
+from . import scheduler as scheduler
+from . import services as services
+from . import utils as utils

Conversation

Gobot1234 commented Dec 18, 2025

Context

Change Summary

Uh oh!

Gobot1234 Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Gobot1234 commented Dec 18, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants