Skip to content

Conversation

mkundu1
Copy link
Contributor

@mkundu1 mkundu1 commented Aug 21, 2025

Context

The common commands at named-objects (like create, rename etc.) will be moved under settings root in settings API. In the first phase, only list and list_properties have been moved in settings API. Those commands will still appear at named-objects from Python code, we have to change their implementation in PyFluent to route them via the root-level commands.

Change Summary

Implemented named-object commands list and list_properties using the root-level commands.

Rationale

The rationale is in the settings API side which is to reduce the size of static-info. The PyFluent side changes are to adapt to that rationale.

Impact

There will be no change in the Python API usage.

@github-actions github-actions bot added the new feature Request or proposal for a new feature label Aug 21, 2025
@mkundu1 mkundu1 force-pushed the feat/named-object-commands branch 2 times, most recently from b0fb800 to 6c4f2bb Compare August 29, 2025 00:16
@mkundu1 mkundu1 force-pushed the feat/named-object-commands branch from e38dadf to 923eb65 Compare September 16, 2025 20:38
@mkundu1 mkundu1 force-pushed the feat/named-object-commands branch from 923eb65 to a6f56be Compare September 24, 2025 15:50
@mkundu1 mkundu1 force-pushed the feat/named-object-commands branch from a6f56be to 00c5092 Compare October 8, 2025 12:15
@mkundu1 mkundu1 force-pushed the feat/named-object-commands branch from 00c5092 to 4305a7e Compare October 8, 2025 16:46
@mkundu1 mkundu1 marked this pull request as ready for review October 8, 2025 17:28
@Copilot Copilot AI review requested due to automatic review settings October 8, 2025 17:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements named-object commands (list and list_properties) using root-level commands to adapt to changes in the settings API that move these commands from named-objects to the settings root. This change reduces static-info size while maintaining backward compatibility.

  • Added list() and list_properties() methods to the NamedObject class that route to root-level commands
  • Implemented parameter name handling to resolve conflicts between method parameters and existing properties
  • Added comprehensive test coverage for both instance methods and static class methods

Reviewed Changes

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

File Description
src/ansys/fluent/core/solver/flobject.py Added list and list_properties methods to NamedObject class and parameter handling for path conflicts
src/ansys/fluent/core/services/settings.py Updated execute_cmd method to handle parameter renaming from path_1 back to path
tests/test_settings_api.py Added test function to verify named-object commands work correctly
doc/changelog.d/4398.added.md Added changelog entry documenting the new feature

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

object_name : str
Name of the object whose properties are to be listed.
"""
# The generated parameter name is path_1 as the name path clashes with existing property.
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment refers to 'generated parameter name' but this is manually written code. The comment should clarify that path_1 is used to avoid conflict with the existing path property, and explain the parameter renaming mechanism.

Suggested change
# The generated parameter name is path_1 as the name path clashes with existing property.
# 'path_1' is used here to avoid a naming conflict with the existing 'path' property.
# When a parameter name would conflict with an existing property, a suffix (e.g., '_1') is added to the parameter name.

Copilot uses AI. Check for mistakes.

Comment on lines +1780 to +1781
if "path" in kwds:
kwds["path_1"] = kwds.pop("path")
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter renaming logic is duplicated in the settings.py file but in reverse. Consider extracting this logic into a utility function to avoid code duplication and ensure consistency.

Copilot uses AI. Check for mistakes.

@seanpearsonuk
Copy link
Collaborator

@gyeole @h-krishnan @mkundu1

Considering the change holistically from the server API onward, could this introduce more complexity for external developers constructing clients, particularly those handling settings APIs generically across products like Fluent, CFX, etc.?

I’m wondering whether moving these commands to the root level makes sense in the broader context of the settings API as a potential cross-product blueprint.

From a trade-off perspective, is the reduction in static info size the primary motivation, or do we expect additional tangible benefits that offset the added indirection?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature Request or proposal for a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants