feat: prep7 submodule

[clatapie]

Description

This PR follows the PyConverter-XML2Py integration plan to automate the PyMAPDL_commands documentation.
The changes have been generated using pyconverter-xml2py and more specifically mapdl-cmd-conv.

This PR focus on the prep7 submodule.

You can check the mapdl-cmd-conv documentation to have a look at the output of each submodules.

Pinging @ansys/pymapdl-developers for visibility. Feel free to provide any feedback on the way the docstrings and the source code generation are handled.

Issue linked

This PR is meant to be merged within the feat/main_commands branch. The latter will gather all the submodule changes, one by one, prior to be merged to the main branch.

Checklist

• I have visited and read Develop PyMAPDL.
• I have tested my changes locally
• I have added necessary documentation or updated existing documentation.
• I have followed the PyMAPDL coding style guidelines.
• I have added appropriate unit tests that also ensure the minimal coverage criteria.
• I have reviewed my changes before submitting this pull request.
• I have linked the issue or issues that are solved to the PR if any.
• I have assigned this PR to myself.
• I have marked this PR as draft if it is not ready to be reviewed yet.
• I have made sure that the title of my PR follows Commit naming conventions (e.g. feat: adding new MAPDL command)

[ansys-reviewer-bot]

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

[germa89]

@clatapie

This PR is HUGE... 37k lines... The previous #3750 was around 5k. Please break this PR into smaller ones, most 4k otherwise, it gets very, very difficult to review it.

[RobPasMue]

@clatapie

This PR is HUGE... 37k lines... The previous #3750 was around 5k. Please break this PR into smaller ones, most 4k otherwise, it gets very, very difficult to review it.

I don't really agree on this @germa89 -- I get that the PR is huge but the purpose of the current PRs is that the content gets autogenerated for a whole module. If the module is this large then so be it... but we shouldn't do a full, thorough review IMO. Rather.. just understand the changes and see what general improvements can be done. We can't be fixing all errors on the first shot sadly.

[germa89]

@clatapie
This PR is HUGE... 37k lines... The previous #3750 was around 5k. Please break this PR into smaller ones, most 4k otherwise, it gets very, very difficult to review it.

I don't really agree on this @germa89 -- I get that the PR is huge but the purpose of the current PRs is that the content gets autogenerated for a whole module. If the module is this large then so be it... but we shouldn't do a full, thorough review IMO. Rather.. just understand the changes and see what general improvements can be done. We can't be fixing all errors on the first shot sadly.

I agree but how I am going to review without reading it all? If the PR is 37k lines, I am not going to do all at once... probably I will do the first 5-7k and then stop. Long reviews are a pain on the web, it gets stuck.
I agree we cannot fix everything at once, but at least we should take note of what is broken (as @clatapie is doing). Otherwise, I think we are not going to revisit these files ever. Once they are merged, we are "done" with them.

So yeah, I still think we should break the PRs in smaller ones.

[clatapie]

IMO, the main goal of these PRs is to check whether the submodules are complete and that each commands are properly working. Indeed, I think we all agree on the fact that the new documentation is more detailed than the previous one. The question is, thus, not about its content but mainly about the structure of the module (= all the functions are well defined with the correct arguments).

For instance, we can notice that there is an issue with the tbft command here, which would have been difficult to catch without this PR. This is a major issue that needs to be fixed before merging the PR.

Also, as you probably noticed, handling every comment is time-consuming, even though some comments just need to be converted into an issue because I always need to check the source of issue. If we want these modules to be published soon, we might need to think of a more scalable strategy. For instance:

• checking the module doesn't miss any information in general
• checking the documentation is understandable and most of the renderings are correct
• if an issue is repeated, open a comment for it to be address
• if an issue is local, add a mention local at the beginning of the comment or directly open an issue on the pyconverter-xml2py

We could then have a more detailed review on the documentation content for the remaining modules.

Please, share your views on those points.
Pinging @germa89, @RobPasMue, @MaxJPRey for visibility.

[RobPasMue]

The question is, thus, not about its content but mainly about the structure of the module (= all the functions are well defined with the correct arguments).

Completely agreed

• checking the module doesn't miss any information in general
• checking the documentation is understandable and most of the renderings are correct

These 2 are the ones we should focus on the review of these PRs.

• if an issue is repeated, open a comment for it to be address
• if an issue is local, add a mention local at the beginning of the comment or directly open an issue on the pyconverter-xml2py

The remaining 2 focus on how to surface the problem. I would either add a comment or open the issue on the converter if it is a common problem

We could then have a more detailed review on the documentation content for the remaining modules.

We should focus on moving forward. Iterations can come afterwards. A minimal good state of these PRs will be better than what we already have for sure.

Thanks for the suggestions @clatapie!

[koubaa]

git will normally track a changed file if it can map the old file to the new one as a modification. In this case, I think because the order of the commands is different, git doesn't realize that this is a move of preproc/areas.py, which makes this code harder to review.

[koubaa]

this is missing the typehint for the integer return

[germa89]

I agree. I guess we could use decorators for this, and add them to the mapdl_extended.py

[koubaa]

I don't see where there are two constraint equation and status files

[koubaa]

One reason the PR is so big and hard to review is that the ordering and naming changed. Otherwise, git will recognize a moved file and only include the changed lines in a diff.

I agree with German that these changes are difficult to review. If there is a code generation change that is simple to review, then it should make very localized changes to the generated code.

For example, if you make a codegen change that adds a typehint to every argument of every command, the codegen system PR can be reviewed alongside a pymapdl PR in which only the typehints are added. That is easy to review, even if it affects 2000 methods.

I would rather see 10s of such PRs than 1 PR combining multiple codegen changes, as that makes it impossible to verify and leans on the test coverage which does not (and would likely never) cover all possible commands.