fix: command conversion - #4126
Merged
Merged
Conversation
Contributor
Reviewer's GuideImplements Python wrappers for the MAPDL FILE and /CLEAR commands with complete docstrings and examples, corrects the MREP command signature to align with APDL syntax, and standardizes documentation by converting literal command mentions to Sphinx cross-references and refining doc formatting across multiple modules. Class diagram for new and updated command wrappers (FILE, CLEAR, MREP)classDiagram
class SetUp {
+clear(read: str = "", **kwargs)
...
}
class SetUpPost1 {
+file(fname: str = "", ext: str = "", **kwargs)
...
}
class SetUpGraphics {
+mrep(name, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18)
...
}
SetUp <|-- SetUpPost1 : uses
SetUp <|-- SetUpGraphics : uses
Class diagram for FILE command wrapper in SetUpPost1classDiagram
class SetUpPost1 {
+file(fname: str = "", ext: str = "", **kwargs)
...
}
Class diagram for CLEAR command wrapper in SetUpclassDiagram
class SetUp {
+clear(read: str = "", **kwargs)
...
}
Class diagram for corrected MREP command signature in SetUpGraphicsclassDiagram
class SetUpGraphics {
+mrep(name, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10, arg11, arg12, arg13, arg14, arg15, arg16, arg17, arg18)
...
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey @clatapie - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- No input validation is performed on fname or ext. (link)
General comments:
- The new
filemethod name shadows the built-in Pythonfileidentifier—consider renaming (e.g.,set_file) to avoid confusion and potential conflicts. - The updated
/MREPsignature removed a placeholder argument—please verify this change matches the official APDL command spec so parameters don’t shift unexpectedly. - After adding
clearandfilecommands, ensure they’re included in the module’s public API (e.g., in__all__or the command registry) so they’re discoverable and autocompletable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `file` method name shadows the built-in Python `file` identifier—consider renaming (e.g., `set_file`) to avoid confusion and potential conflicts.
- The updated `/MREP` signature removed a placeholder argument—please verify this change matches the official APDL command spec so parameters don’t shift unexpectedly.
- After adding `clear` and `file` commands, ensure they’re included in the module’s public API (e.g., in `__all__` or the command registry) so they’re discoverable and autocompletable.
## Individual Comments
### Comment 1
<location> `src/ansys/mapdl/core/_commands/post1/set_up.py:629` </location>
<code_context>
+ POST26
+ ^^^^^^
+ """
+ command = f"FILE,{fname},{ext}"
+ return self.run(command, **kwargs)
+
</code_context>
<issue_to_address>
No input validation is performed on fname or ext.
Special characters in these inputs could lead to malformed APDL commands or injection risks. Please add validation or sanitization to ensure safe input values.
Suggested implementation:
```python
import re
def file(self, fname: str = "", ext: str = "", **kwargs):
r"""Specifies the data file where results are to be found.
Mechanical APDL Command: `FILE <https://ansyshelp.ansys.com/Views/Secured/corp/v232/en//ans_cmd/Hlp_C_FILE.html>`_
Parameters
----------
fname : str
File name and directory path (248 characters maximum, including the characters needed for the
directory path). An unspecified directory path defaults to the working directory; in this case,
Only alphanumeric characters, underscores, hyphens, and periods are allowed. Directory separators are allowed.
ext : str
File extension. Only alphanumeric characters and underscores are allowed.
Raises
------
ValueError
If `fname` or `ext` contains invalid characters.
```
```python
# Allow alphanumerics, underscores, hyphens, periods, and directory separators in fname
if not re.match(r'^[\w\-.\\/]*$', fname):
raise ValueError(
f"Invalid characters in fname: {fname!r}. Only alphanumerics, underscores, hyphens, periods, and directory separators are allowed."
)
# Allow only alphanumerics and underscores in ext
if not re.match(r'^[\w]*$', ext):
raise ValueError(
f"Invalid characters in ext: {ext!r}. Only alphanumerics and underscores are allowed."
)
command = f"FILE,{fname},{ext}"
return self.run(command, **kwargs)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
germa89
enabled auto-merge (squash)
July 29, 2025 15:25
germa89
reviewed
Jul 30, 2025
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4126 +/- ##
==========================================
- Coverage 91.35% 91.30% -0.05%
==========================================
Files 189 189
Lines 15650 15657 +7
==========================================
- Hits 14297 14296 -1
- Misses 1353 1361 +8 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes a few issues related to the conversion of the the MAPDL documentation.
Concerned functions:
clearfilemrepChecklist
draftif it is not ready to be reviewed yet.feat: adding new MAPDL command)Summary by Sourcery
Add clear() and file() methods, fix mrep argument formatting, and modernize documentation by using Sphinx cross-reference syntax for APDL commands across the codebase
New Features:
Bug Fixes:
Enhancements: