ansys / ansys/pyfluent

Type checking the settings API

Open
#4,839 2 comments 1 reaction 0 assignees View on GitHub
enhancement
Dominant language
Python
Stars
497
Forks
77
Avg merge
22h 37m
Merged PRs (30d)
45

Description

I've been staring at this for a bit of time now and I've noticed a couple of problems (might add more as I go):

- The files are too large currently for type checkers to bother with them, can we separate them into smaller submodules? Perhaps on a per setting basis or per setup/model/physics? Image
- No method return types are typed, e.g. create should return Self, list_x returns a list[str] etc. Optional parameters also aren't marked as such with their defaults in the stubs.
- `settings_builtin.pyi` the base classes list seems wrong for all of the classes here making them practically unusable
```py
from ansys.fluent.core.generated.solver.settings_262 import root as settings_root_262
from ansys.fluent.core.generated.solver.settings_261 import root as settings_root_261
from ansys.fluent.core.generated.solver.settings_252 import root as settings_root_252
from ansys.fluent.core.generated.solver.settings_251 import root as settings_root_251
from ansys.fluent.core.generated.solver.settings_242 import root as settings_root_242


class Setup(
type(settings_root_262.setup),
type(settings_root_261.setup),
type(settings_root_252.setup),
type(settings_root_251.setup),
type(settings_root_242.setup),
): ...
```
It needs to use the type directly (resolve the .setup attribute and get it's type)
The parameters to init these as well are also wrong, they show up as (name, parent) when they roughly accept (solver, name)

- `__set__` seems missing from the child classes even though this is supported (it writes to .value) (see https://github.com/ansys/pyfluent-visualization/pull/627/changes#diff-29069ded6a954bcdbdd198b845855ab7297b91d501ef59d62c36791612b18ac2R340-R342 for how I did this) I think this just requires changes in flobject.py
- RealType should be turned into a union not a NewType (it'll be extremely cumbersome to use any API if you need to use RealType(x) every time)
- The rest of these types should probably use their covariant counterparts (excluding the vector tuple, though I wonder if this is missing the 2d case)
- Some classes seem to not be defined correctly e.g.
```py
controls = Controls(solver)

# Disable flow and turbulence equations
controls.equations["flow"] = False # type error "__setitem__" method not defined on type "equations"
controls.equations["kw"] = False
```
Image
- Enums aren't typed as str subclasses with the is_active method

Contributor guide

Open the contributing guide

Research direction

Start by reading flobject.py and settings_builtin.pyi, then compare the generated solver settings modules named in the issue. The work is complete when the settings API stubs consistently represent return and optional types, inheritance and constructors, child assignment, collection updates, scalar types, and enums.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.