Fix issue 4206 #4207
Fix issue 4206 #4207
Conversation
|
All Contributor License Agreement (CLA) signatures have been captured successfully. Thanks for contributing! |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #4206 by adding handling for "C_FullFile" parameter types in the parameter status interpretation function. The fix prevents errors when encountering C_FullFile parameters that don't follow the standard parameter structure.
- Adds specific handling for "C_FullFile" parameter type in the
interp_star_statusfunction - Creates a dictionary entry with only the type field for C_FullFile parameters, avoiding shape parsing issues
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| "type": items[1], | ||
| "workspace": int(items[4]), | ||
| } | ||
| elif items[1] in ["C_FullFile"]: |
There was a problem hiding this comment.
Using a list with a single element for string comparison is unnecessary. Consider using a direct string comparison: elif items[1] == "C_FullFile": for better readability and performance.
| elif items[1] in ["C_FullFile"]: | |
| elif items[1] == "C_FullFile": |
| "type": items[1], | ||
| "workspace": int(items[4]), | ||
| } | ||
| elif items[1] in ["C_FullFile"]: |
There was a problem hiding this comment.
The dictionary structure is inconsistent with other parameter types above, which include additional fields like 'workspace' or 'shape'. Consider adding a comment explaining why C_FullFile parameters only need the type field, or ensure consistency with the expected parameter structure.
| elif items[1] in ["C_FullFile"]: | |
| elif items[1] in ["C_FullFile"]: | |
| # C_FullFile parameters only require the 'type' field. | |
| # No additional fields (e.g., 'workspace', 'shape') are needed for this type. |
|
@Revathyvenugopal162 the CLA bot is failing with with a wrong user it seems. @ChaiF1 did you sign the contributors agreement? https://cla.developer.ansys.com |
|
@ChaiF1 do you want to add some unit tests to this? Or shall I do it? |
@german, in this PR as well, there is an orphan commit, may be because the cla is failing. i m not sure why it is orphan |
|
Thank you for your contribution @ChaiF1! You are going through the CLA process because you are considered as an external contributor. Could you please join the Ansys organization? You can reach out to me if you need any support with that. |
@germa89 Hi, sorry I forgot to check the status on this PR for a few days. I signed the CLA once I made this branch, so not sure what is going wrong. I am also in the ANSYS organisation since today. Ill check briefly tommorow to see if I can fix the issue of the orphaned commit. As for the unit tests, could you take care of those? I don't really have the knowledge of the codebase to implement those. |
|
Bonjour. After looking around a bit I think that the issues with the orphaned commit are solved? I am not sure as I am extremely experienced with git, I only know the basics. If you need anything else from me just let me know! |
Yes, the cla bot is passing. |
|
Since this branch is comming from a fork, we will have to migrate it. @pyansys-ci-bot migrate |
|
❌ Error ❌ An error occurred while migrating or syncing the PR. Pinging @pymapdl-maintainers for assistance. |
🚀 Migration completed!The pull request #4279 has been created successfully. Thank you @ChaiF1 for your contribution! Please review the new PR and make any necessary changes. This PR will be closed by the admins soon. You can now start to work in the new pull request by cloning this pull request branch in a new repository by doing: If you have GitHub CLI installed, you can also use the following command to check out the pull request: Thank you again for your contribution @ChaiF1! |
5 participants
This is a small PR to give the code I used to fix issue #4206 which I encountered in my work. The issue is broader than my fix so more code is needed to make the problems described in the issue go away. This can be used as a start to start fixing the entire underlying problem.