ansys / pyfluent

Pythonic interface to Ansys Fluent
https://fluent.docs.pyansys.com
MIT License
254 stars 42 forks source link

Can we make allowed_values usage consistent between PyFluent and PyFluent Visualization? #728

Closed ypatel-qa closed 2 years ago

ypatel-qa commented 2 years ago

πŸ“ Description of the feature

allowed_values usage is not consistent between PyFluent and PyFluent Visualization.

PyFluent:

In [14]: session.solver.root.setup.models.viscous.model.get_attr('allowed-values') Out[14]: ['inviscid', 'laminar', 'k-epsilon-standard', 'k-omega-standard', 'mixing-length', 'spalart-allmaras', 'k-kl-w', 'transition-sst', 'reynolds-stress', 'scale-adaptive-simulation', 'detached-eddy-simulation', 'large-eddy-simulation']

PyFluent Visualization:

In [10]: mesh1.surfaces_list.allowed_values Out[10]: ['in1', 'in2', 'in3', 'out1', 'solid_up:1:830', 'solid_up:1', 'inlet', 'inlet1', 'inlet2', 'outlet', 'solid_up:1:830-shadow']

πŸ’‘ Steps for implementing the feature

Try get_attr('allowed-values') in the PyFluent and try surfaces_list.allowed_values in the PyFluent Visualization.

Just allowed_values without get_attr(), like what we have in the PyFluent Visualization, is easier to follow. Can we support allowed_values usage everywhere if possible?

πŸ”— Useful links and references

No response

seanpearsonuk commented 2 years ago

Settings API and meshing workflow (hence all data model based APIs) now have explicit attribute accessors. e.g. we have obj.allowed_values() on settings objects. For the above visualization example allowed_values is just a property so we are not completely aligned.

We could make all those new methods _property_s. That would be fully aligned. @prmukherj @ypatel-qa

seanpearsonuk commented 2 years ago

Spoke with @prmukherj we agree that APIs are all OK right now. Possibly we could change the allowed_values property in visualization to become a method, but it's not a big issue @ajain-work - we could keep it as is because logically it is a property

seanpearsonuk commented 2 years ago

we're going to make everything a property on the "API" side