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.
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
DOC: add section on how to write plugin #463
DOC: add section on how to write plugin #463
Changes from 20 commits
2987b65
a07f39e
04f94b0
013ede3
2c707f4
ef54a03
81ce871
3314b0b
6a29f4f
9636663
ed7fb61
60f7480
413b7ff
7ddf675
58cc5e2
9e4b25f
6cdf2cb
a9a767c
7dd452b
cf0fd50
5b5bab9
f766a42
9602b18
fa0010f
f0718d6
710848d
ad25ddc
1dcd6d0
546e0d0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This example is not very clear, in the text it says "when the user selects to run the properties." but here the properties are not shown. Could you include the properties list and maybe using two figures to show the differents before and after selecting a new property?
Only if you agree with my suggestion, I think PS and replot the figures are tedious and may not make things better, I'll pining @mikibonacci for comments to see which way is better.
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 will vote for Hello world (if we do not use MultiplyAdd instead) and for the gif, as it will be more dynamic and less tedious.
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.
Sorry for being late, I am pro Hello world since this is more a feature for developers, though i would be more incline to specify something like "Plugin: Custom Property" or "Plugin: Your New Feature" (or property) i think is more related to the idea that each plugin is linked to a new 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.
Can you make the description of the plugin the line "A aiidalab-qe plugin to print hello world" indent properly. I know it is the description, but seems like an isolated line. Maybe put a border on the widget?
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 removed this description line from the plugin.
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.
Super cool!
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.
Is this a rule for developers or it is a thing to note?
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 changed it to
This integration is made possible due to several key aspects:
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.
See my comment on this file, I think it maybe better to have two static figures to show before and after the property being selected?
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.
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 prefer to keep
typically
, and not addcorresponding
.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.
A bit confusing to me. Does a panel register a new panel? Can you be more specific?
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 changed it to
A setting Panel widget will register a new panel in the configuration step
.In the feature, we may create a
SettingPanel
class for the setting, like theResultPanel
.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.
Same here, maybe "register" -> "render"
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 prefer to use
register
. The pluginregisters
(adds) a new panel into the App. The web browser and Jupyterrender
the panel in the GUI.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.
In my opinion, "hello world" example is used very specifically for programming language, I guess even aiida plugin didn't use a "hello world" example. Here more specific name could be better. Please consider to change it.
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.
should we use the "MultiplyAddWorkChain" from AiiDA tutorial? https://aiida.readthedocs.io/projects/aiida-core/en/latest/intro/tutorial.html . This is just a detail, but maybe we can make the very same example.