Skip to content
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

Update plugin howto #5000

Merged
merged 20 commits into from
Jul 3, 2021
Merged

Update plugin howto #5000

merged 20 commits into from
Jul 3, 2021

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Jun 28, 2021

No description provided.

@unkcpz
Copy link
Member

unkcpz commented Jun 30, 2021

Hi @ltalirz , you may want to have look at #5001 along with this PR.

…pler, moved all parser related methods into one file and small changes to documentation - mostly imports
@ltalirz ltalirz marked this pull request as ready for review June 30, 2021 21:48
Copy link
Member Author

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @sudarshanv01 !

I've gone through the PR now; a couple of change requests

aiida/calculations/diff/diff_calculation.py Outdated Show resolved Hide resolved
aiida/calculations/diff/diff_calculation.py Outdated Show resolved Hide resolved
aiida/parsers/plugins/diff/diff_parser.py Outdated Show resolved Hide resolved
aiida/parsers/plugins/diff/diff_parser.py Outdated Show resolved Hide resolved

:returns: an exit code, if parsing fails (or nothing if parsing succeeds)
"""
from aiida.orm import SinglefileData
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the top

docs/source/howto/plugin_codes.rst Outdated Show resolved Hide resolved
docs/source/howto/plugin_codes.rst Outdated Show resolved Hide resolved
docs/source/howto/plugin_codes.rst Outdated Show resolved Hide resolved
docs/source/howto/plugin_codes.rst Outdated Show resolved Hide resolved
Comment on lines 454 to 455
from aiida.engine import submit
result = engine.submit(DiffCalculation, **builder)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original code is the correct one and this change should be reverted

@ltalirz
Copy link
Member Author

ltalirz commented Jun 30, 2021

Also note that there are currently pre-commit and docs build failures

sudarshanv01 and others added 3 commits July 2, 2021 08:32
Co-authored-by: Leopold Talirz <leopold.talirz@gmail.com>
Co-authored-by: Leopold Talirz <leopold.talirz@gmail.com>
spec.inputs['metadata']['options']['resources'].default = {
'num_machines': 1,
'num_mpiprocs_per_machine': 1,
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not being autoformatted via the pre-commit hooks?
if it was, this would almost certainly change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, when modifying the code of the python scripts, please always check the corresponding text in the tutorial.
e.g. in this case the text expects these two lines to be after spec.output.

@ltalirz ltalirz enabled auto-merge (squash) July 3, 2021 11:34
@codecov
Copy link

codecov bot commented Jul 3, 2021

Codecov Report

Merging #5000 (b060742) into develop (9c7ffc2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5000   +/-   ##
========================================
  Coverage    80.13%   80.13%           
========================================
  Files          515      515           
  Lines        36692    36692           
========================================
  Hits         29399    29399           
  Misses        7293     7293           
Flag Coverage Δ
django 74.60% <ø> (ø)
sqlalchemy 73.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c7ffc2...b060742. Read the comment docs.

@ltalirz ltalirz disabled auto-merge July 3, 2021 21:37
@ltalirz ltalirz merged commit 390ea5b into aiidateam:develop Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants