Type checking the settings API
- 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?
- 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
```
- Enums aren't typed as str subclasses with the is_active method
Contributor 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