Conversation
…nto feat/enhanced-cli
SMoraisAnsys
left a comment
SMoraisAnsys
left a comment
There was a problem hiding this comment.
Can you update the PR description so that it helps reviewing the changes ?
Ready to review |
SMoraisAnsys
left a comment
SMoraisAnsys
left a comment
There was a problem hiding this comment.
Thanks for the changes @eblanco-ansys Here are some minor comments
| pip install pyaedt[all] | ||
| The script runs with an attached Desktop object available as ``desktop``. | ||
|
|
||
| **Run am Ironpython script** |
There was a problem hiding this comment.
| **Run am Ironpython script** | |
| **Run an Ironpython script** |
Do we really want to support the run of Iron Python scripts ? Shouldn't we establish a clear line between pyaedt and IronPython ?
There was a problem hiding this comment.
I had this conversation yesterday with @Samuelopez-ansys and it's opened to discussion. I also agree with your point
There was a problem hiding this comment.
I think it is useful, there are still users with Ironpython scripts. But if you do not agree we can delete it
| return "ansysedtsv.exe" if student_version else "ansysedt.exe" | ||
|
|
||
|
|
||
| def _discover_aedt_sessions() -> list[dict[str, object]]: |
There was a problem hiding this comment.
Could you reuse the internal logic that we have to handle this ? I mean, we have some internal logic to parse command line and so on so this would help with future maintenance if it's needed.
| @session_app.command("list") | ||
| def list_sessions() -> None: | ||
| """List all running AEDT instances.""" | ||
| aedt_sessions = _discover_aedt_sessions() |
There was a problem hiding this comment.
Could you tell me why some commands are in try: ... except: ... while others don't ? Should this be homogenized or is this on purpose ?
| return designs | ||
|
|
||
|
|
||
| def get_active_design_name(project) -> str | None: |
There was a problem hiding this comment.
Same comment as before, if that's not already the case, could we have some "internal" modules where this kind of code is stored ? AFAIK this kind of call (and the following) are used in other places like the extensions, ... It would be better if we could have a single location for everything.
| CONFIG_DESCRIPTIONS = { | ||
| "desktopVersion": "AEDT version to use", | ||
| "NonGraphical": "Run AEDT without GUI", | ||
| "NewThread": "Use new thread for AEDT", | ||
| "skip_circuits": "Skip circuit tests", | ||
| "use_grpc": "Use gRPC for communication", | ||
| "close_desktop": "Close AEDT after tests", | ||
| "use_local_example_data": "Use local example data", | ||
| "local_example_folder": "Path to local examples", | ||
| "skip_modelithics": "Skip Modelithics tests", | ||
| "use_pyedb_grpc": "Use PyEDB with gRPC for database access", | ||
| } |
There was a problem hiding this comment.
I think this idea was already raised but could we align the configuration keys to lower case everywhere at some point ? This might be done in another PR ofc but I wanted to highlight it as I'm reading it.
There was a problem hiding this comment.
I think this idea was already raised but could we align the configuration keys to lower case everywhere at some point ? This might be done in another PR ofc but I wanted to highlight it as I'm reading it.
I agree, we should definitely do this in another PR. I think it's the moment to lower case all of them. @Samuelopez-ansys thoughts on this?
There was a problem hiding this comment.
Indeed, in a new PR we can do it.
4 participants
Description
This pull request introduces significant enhancements and refactoring to the CLI infrastructure for PyAEDT, with a focus on expanding available commands, improving modularity, and providing better support for test configuration management. The changes include new CLI commands, a more structured approach to sub-app registration, and the addition of agent-friendly JSON output. Several utility functions in
common.pyhave been refactored for clarity and reusability, and process management code has been removed fromcommon.pyto streamline responsibilities.CLI Command and Structure Enhancements:
session,project,script, andexport, and restructured the main CLI app to use a callback for JSON output and improved help output. Also, added new commandsversionandaedt-versionsfor version reporting. (src/ansys/aedt/core/cli/__init__.py)src/ansys/aedt/core/cli/__main__.pyto enable running the CLI as a module (e.g.,python -m ansys.aedt.core.cli).doc/changelog.d/7549.added.md)JSON Output and Utility Improvements:
json_modeflag and aprint_outputfunction incommon.pyto support agent-friendly JSON output for CLI commands. (src/ansys/aedt/core/cli/common.py)src/ansys/aedt/core/cli/common.py)These changes collectively improve the CLI's modularity, maintainability, and usability, enabling future agentic integrations using PyAEDT CLI**
Checklist