CLI: verdi export to a yaml file with keys from code class #5860

unkcpz · 2023-01-06T16:37:22Z

Addresses #3521 partially.

This is mentioned in usability improvement as well as having a command to export the code and computer setup.
Keys of YAML file are read from the cli option of the corresponding code class.

However, for the prepend_text and append_text I did not implement polish of the yaml output. If they are multiline content, the value of the text in YAML file won't be properly indented. For example, see the output YAML below:

append_text: ''
computer: localhost
default_calc_job_plugin: core.templatereplacer
description: doubler
filepath_executable: /min
label: doubler1
prepend_text: 'module load daint-gpu

    module load CP2K

    '
use_double_quotes: 'False'

where the prepend_text of original config yaml file is:

prepend_text: |
    module load daint-gpu
    module load CP2K

When I look at #3616, I find @sphuber tried to put into subcommand show format options to dump as YAML. It can also be an option.

unkcpz · 2023-01-06T16:38:36Z

A mypy error found, how can I fix it? @chrisjsewell

aiida/orm/nodes/data/code/abstract.py:153: error: Property "label" defined in "BackendNode" is read-only  [misc]
Found 1 error in 1 file (checked 2 source files)

chrisjsewell · 2023-01-11T09:56:31Z

A mypy error found, how can I fix it?

I don't see this on the actual CI pre-commit run?
Perhaps you need to run against all files: pre-commit run --all mypy

unkcpz · 2023-01-11T12:03:05Z

I don't see this on the actual CI pre-commit run?

Ah, my bad. I didn't check the failed pre-commit action very carefully. It is different from I run mypy locally. I know how to fix other issues that fail the pre-commit action. Thanks for help!

sphuber

Thanks @unkcpz , couple comments. As for having a separate command to export, or an option to verdi code show, I would be ok with both, but I think the choice of this PR might be better in hindsight. Seems more explicit and more easy to find.

sphuber · 2023-01-16T13:43:22Z

tests/cmdline/commands/test_code/test_code_exportyml

+filepath_executable: /bin/cat
+label: code
+prepend_text: ''
+use_double_quotes: 'False'


Will this be correctly interpreted? Should this not be a literal false or at least without quotes? Ideal would be to have a round-trip test and call verdi code create with this yaml and see if you get equivalent codes

Yes, the False as a string is working, I add a round trip test for it.

sphuber · 2023-01-16T13:44:49Z

aiida/cmdline/commands/cmd_code.py

+        else:
+            value = getattr(code, key)
+
+        code_data[key] = str(value)


Do we need to convert to str? Should the yaml serializer not take care of this. Then the boolean question would be solved in any case. I don't think we have codes that have any attributes that are not YAML-serializable. I think the data is guaranteed to be JSON-serializable, since it comes from the database, so it should also be YAML-serializable.

The problem comes from the filepath_executable which I read by code._get_cli_options. The type is PurePosixPath which is not yaml-serializable. I can make another elif for this, but I think should be fine that all converted to string?

Yeah I agree, fine to keep the cast to str then as long as the round-trip works

sphuber · 2023-01-16T13:45:05Z

aiida/cmdline/commands/cmd_code.py

+    cli_options = code._get_cli_options()
+    code_data = {}
+
+    for key in cli_options.keys():


Suggested change

cli_options = code._get_cli_options()

code_data = {}

for key in cli_options.keys():

code_data = {}

for key in code._get_cli_options().keys():

sphuber · 2023-01-16T13:45:36Z

aiida/cmdline/commands/cmd_code.py

+        code_data[key] = str(value)
+
+    with open(output_file, 'w') as fh:
+        fh.write(yaml.dump(code_data, indent=4))


Since the data structure is flat, the indent seems superfluous

Suggested change

fh.write(yaml.dump(code_data, indent=4))

fh.write(yaml.dump(code_data))

unkcpz · 2023-01-17T16:47:34Z

Hi @sphuber, I add a test to setup from the file generated. Fix the pre-commit warning by rename the _get_cli_option to get_cli_option. Do you think reading from get_cli_options is a good way? since it is a class function but I am here calling it from the object.

sphuber · 2023-01-17T16:48:42Z

aiida/orm/nodes/data/code/abstract.py


    @classmethod
-    def _get_cli_options(cls) -> dict:


This change is incorrect. The idea is that it is private and that subclasses override this. The base class already defines the public get_cli_options which should be used by users of this class

Ah, I didn't notice that, shame failure. Thanks!

sphuber · 2023-01-19T09:30:22Z

tests/cmdline/commands/test_code.py

+    )
+    new_code = load_code(new_label)
+    assert new_code.use_double_quotes is False
+    assert isinstance(load_code(new_label), InstalledCode)


Suggested change

assert isinstance(load_code(new_label), InstalledCode)

assert isinstance(new_code, InstalledCode)

sphuber · 2023-01-19T09:31:52Z

tests/cmdline/commands/test_code.py

+        cmd_code.code_create, ['core.code.installed', '--non-interactive', '--config', filepath, '--label', new_label]
+    )
+    new_code = load_code(new_label)
+    assert new_code.use_double_quotes is False


I take it that assert code == new_code won't work since the AbstractCode doesn't implement the __eq__ method. But maybe instead of comparing just this one attribute arbitrarily, we do assert code.base.attributes.all() == new_code.base.attributes.all()? That should cover at least all attributes, which for at least the InstalledCode should be all relevant data.

sphuber

Thanks @unkcpz some final minor comments

sphuber · 2023-01-19T09:33:28Z

tests/cmdline/commands/test_code.py

@@ -236,6 +236,24 @@ def test_code_duplicate_ignore(run_cli_command, aiida_local_code_factory, non_in
    assert duplicate.description == ''


+def test_code_export(run_cli_command, aiida_local_code_factory, tmp_path, file_regression):
+    """Test export the code setup to str."""
+    code = aiida_local_code_factory('core.arithmetic.add', '/bin/cat', label='code')


Can you please add some data for append_text and prepend_text. Ideally some multiline string with maybe some leading whitespace. So e.g.:

prepend_text = 'module load something\n some command'

Yes, added.

fixes aiidateam#3521 Add `verdi code export` command to export code from command line as a ymal file. This is mentioned in usability improvement as well as having a command to export the code and computer setup. Keys of YAML file are read from the cli option of the corresponding code class.

unkcpz · 2023-01-22T08:20:16Z

Hi @sphuber, I also prepare an AEP draft for how I want to tackle the computer/code setup in the end, aiidateam/AEP#37.

sphuber

Thanks for the changes @unkcpz , good to go now

sphuber requested changes Jan 16, 2023

View reviewed changes

unkcpz force-pushed the fea/xx/export-code-setup branch from 8481604 to 210b1f7 Compare January 17, 2023 16:45

unkcpz requested a review from sphuber January 17, 2023 16:46

sphuber reviewed Jan 17, 2023

View reviewed changes

unkcpz force-pushed the fea/xx/export-code-setup branch from 210b1f7 to d646cdb Compare January 17, 2023 17:28

sphuber reviewed Jan 19, 2023

View reviewed changes

sphuber requested changes Jan 19, 2023

View reviewed changes

unkcpz force-pushed the fea/xx/export-code-setup branch from f5dd578 to 48d4e0b Compare January 21, 2023 22:58

unkcpz changed the title ~~export to a yaml file with keys from code class~~ CLI: verdi export to a yaml file with keys from code class Jan 21, 2023

unkcpz requested a review from sphuber January 22, 2023 08:16

Merge branch 'main' into fea/xx/export-code-setup

1aca65b

sphuber approved these changes Jan 30, 2023

View reviewed changes

sphuber merged commit a6f3d60 into aiidateam:main Jan 30, 2023

unkcpz deleted the fea/xx/export-code-setup branch May 26, 2023 09:40

sphuber mentioned this pull request Jun 1, 2024

dump computer/codes in YAML format #3521

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI: verdi export to a yaml file with keys from code class #5860

CLI: verdi export to a yaml file with keys from code class #5860

unkcpz commented Jan 6, 2023 •

edited by sphuber

Loading

unkcpz commented Jan 6, 2023 •

edited

Loading

chrisjsewell commented Jan 11, 2023

unkcpz commented Jan 11, 2023

sphuber left a comment •

edited

Loading

sphuber Jan 16, 2023

unkcpz Jan 17, 2023

sphuber Jan 16, 2023

unkcpz Jan 17, 2023

sphuber Jan 19, 2023

sphuber Jan 16, 2023

sphuber Jan 16, 2023

unkcpz commented Jan 17, 2023 •

edited

Loading

sphuber Jan 17, 2023

unkcpz Jan 17, 2023

sphuber Jan 19, 2023

sphuber Jan 19, 2023

sphuber left a comment

sphuber Jan 19, 2023

unkcpz Jan 22, 2023

unkcpz commented Jan 22, 2023

sphuber left a comment

	fh.write(yaml.dump(code_data, indent=4))
	fh.write(yaml.dump(code_data))

	assert isinstance(load_code(new_label), InstalledCode)
	assert isinstance(new_code, InstalledCode)

CLI: verdi export to a yaml file with keys from code class #5860

CLI: verdi export to a yaml file with keys from code class #5860

Conversation

unkcpz commented Jan 6, 2023 • edited by sphuber Loading

unkcpz commented Jan 6, 2023 • edited Loading

chrisjsewell commented Jan 11, 2023

unkcpz commented Jan 11, 2023

sphuber left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unkcpz commented Jan 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sphuber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unkcpz commented Jan 22, 2023

sphuber left a comment

Choose a reason for hiding this comment

unkcpz commented Jan 6, 2023 •

edited by sphuber

Loading

unkcpz commented Jan 6, 2023 •

edited

Loading

sphuber left a comment •

edited

Loading

unkcpz commented Jan 17, 2023 •

edited

Loading