-
Notifications
You must be signed in to change notification settings - Fork 142
fix: Added missing kwargs to the dmpoption command. #4027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thanks for opening a Pull Request. If you want to perform a review write a comment saying: @ansys-reviewer-bot review |
Reviewer's GuideThe PR enhances the existing dmpoption method by introducing two new keyword arguments corresponding to documented options, updating its docstring, and altering the command string builder to pass these parameters. Class diagram for updated dmpoption methodclassDiagram
class SolutionCommands {
+dmpoption(filetype="", combine="", rescombfreq="", deleopt="", **kwargs)
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR adds two missing keyword arguments (rescombfreq, deleopt) to the dmpoption command to align with the official ANSYS documentation.
- Extended
dmpoptionsignature withrescombfreqanddeleopt - Updated docstring to describe the new parameters
- Modified the command string assembly to include the new arguments
Comments suppressed due to low confidence (1)
src/ansys/mapdl/core/_commands/solution/analysis_options.py:1369
- A comma is missing between
{combine}and{rescombfreq}. It should be...,{combine},{rescombfreq},{deleopt}to correctly separate all arguments.
command = f"DMPOPTION,{filetype},{combine}{rescombfreq},{deleopt}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mbbatukan - I've reviewed your changes - here's some feedback:
- The f-string for DMPOPTION is missing a comma between the combine and rescombfreq fields (should be
...,{combine},{rescombfreq},...). - Consider adding validation or enum checks for the new kwargs (rescombfreq, deleopt) to restrict them to the supported ANSYS keyword values.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The f-string for DMPOPTION is missing a comma between the combine and rescombfreq fields (should be `...,{combine},{rescombfreq},...`).
- Consider adding validation or enum checks for the new kwargs (rescombfreq, deleopt) to restrict them to the supported ANSYS keyword values.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
germa89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mbbatukan! Thanks you a lot for your contribution! It is greatly appreciated! 😄
Since you are trying to merge from a fork, and the forks in GitHub do not have access to the repository secrets needed to run the CICD pipelines, your PR needs to be migrated to another PR within PyMAPDL repository. Do not worry this is an automated process, which the bot and I will perform. If there are any other changes needed in this PR to make the CICD pass, you can make them in that branch or this one, I will take care of syncing both. Ping me if I take too long to reply or show up.
The new PR keeps the commit history, so you will be acknowledged as PyMAPDL contributor even if the migrated PR is made by the bot or myself.
Again, thank you a lot for your contribution! 🚀
|
@pyansys-ci-bot migrate |
|
❌ Error ❌ An error occurred while migrating or syncing the PR. Pinging @pymapdl-maintainers for assistance. |
2 similar comments
|
❌ Error ❌ An error occurred while migrating or syncing the PR. Pinging @pymapdl-maintainers for assistance. |
|
❌ Error ❌ An error occurred while migrating or syncing the PR. Pinging @pymapdl-maintainers for assistance. |
🚀 Migration completed!The PR #4030 has been created successfully. Thank you @mbbatukan for your contribution! Please review the PR and make any necessary changes. If you have GitHub CLI installed, you can also use the following command to check out the pull request: This PR will be closed by the admins when the new PR has been checked to work. |
Description
Please provide a brief description of the changes made in this pull request.
Added missing kwargs to the dmpoption command.
Issue linked
Please mention the issue number or describe the problem this pull request addresses.
It is a very minor fix. Please see the changes and refer to the documentation: https://ansyshelp.ansys.com/public/account/secured?returnurl=/Views/Secured/corp/v242/en/ans_cmd/Hlp_C_DMPOPTION.html
Summary by Sourcery
Add missing rescombfreq and deleopt parameters to the dmpoption command to enable configuring result combination frequency and local file deletion.
Bug Fixes:
Documentation: