-
Notifications
You must be signed in to change notification settings - Fork 123
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
Documentation cleanup #533
Conversation
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.
LGTM. I saw a couple of spelling mistakes, which I've already corrected and also made a suggestion about non_interactive, but that might be worth going in a separate issue rather than being a focus of this PR.
@@ -145,10 +144,34 @@ def __init__(self, loglevel='DEBUG', use_vtk=True, log_apdl=False, | |||
|
|||
self._post = PostProcessing(self) | |||
|
|||
@property |
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.
Would this be better as a function? I think it's already a property, so this might be a breaking change, but seeing with
for a property looks quite odd to me, maybe it's also the lack of an as thing
as well!
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.
I agree that it's a bit odd, I was trying for simplicity here:
with mapdl.non_interactive:
mapdl.prep7()
...
vs a more standard approach:
with mapdl.non_interactive() as ni:
ni.prep7()
...
However, the former approach is non-standard and I'll agree that it has side effects (since it's effectively changing the state of the mapdl
object). Let's consider changing this in a future PR.
This PR cleans up the documentation by:
Query
class documentation to the API section.Also edits the CI with: