Several tests write to node.open

[ltalirz]

E.g.

    def test_node_repo_cat(self):
        """Test 'verdi node repo cat' command."""
        # Test cat binary files
        with self.folder_node.open('filename.txt.gz', 'wb') as fh_out:
            fh_out.write(gzip.compress(b'COMPRESS'))

Resulting in warnings

tests/cmdline/commands/test_calcjob.py::TestVerdiCalculation::test_calcjob_inputcat
tests/cmdline/commands/test_calcjob.py::TestVerdiCalculation::test_calcjob_outputcat
tests/cmdline/commands/test_node.py::TestVerdiNode::test_node_repo_cat
tests/engine/test_calc_job.py::test_parse_not_implemented
tests/engine/test_calc_job.py::test_parse_scheduler_excepted
  /home/runner/work/aiida-core/aiida-core/aiida/orm/nodes/node.py:452: AiidaDeprecationWarning: from v2.0 only the modes 'r' and 'rb' will be accepted
    warnings.warn("from v2.0 only the modes 'r' and 'rb' will be accepted", AiidaDeprecationWarning)  # pylint: disable=no-member

How should these tests be rewritten?

[giovannipizzi]

The issue is that here we are hacking into an already stored node (self.folder_node) and adding a file without going via the ORM API. This is still working now (opening a file just checks if the file is there) but will not work in 2.0 (one also has to register that the file is there in the DB in 2.0).

The solution I'm implementing is the following. First, I don't have a pre-stored node self.folder_node, but a "factory" function

    @classmethod
    def get_unstored_folder_node(cls):
        """Get a "default" folder node with some data.

        The node is unstored so one can add more content to it before storing it.
        """
        folder_node = orm.FolderData()
        cls.content_file1 = 'nobody expects'
        cls.content_file2 = 'the minister of silly walks'
        cls.key_file1 = 'some/nested/folder/filename.txt'
        cls.key_file2 = 'some_other_file.txt'
        folder_node.put_object_from_filelike(io.StringIO(cls.content_file1), cls.key_file1)
        folder_node.put_object_from_filelike(io.StringIO(cls.content_file2), cls.key_file2)
        return folder_node

Then:

1. In the tests where I just needed to use self.folder_node without modifying, I just add at the top folder_node = self.get_unstored_folder_node().store() and reference simply folder_node rather than self.folder_node
2. In the tests where I need to add content (like the one above), I do something like this:

    def test_node_repo_cat(self):
        """Test 'verdi node repo cat' command."""
        # Test cat binary files
        folder_node = self.get_unstored_folder_node()
        folder_node.put_object_from_filelike(io.BytesIO(gzip.compress(b'COMPRESS')), 'filename.txt.gz', mode='wb')
        folder_node.store()

[ltalirz]

Just came back to this issue due to a from v2.0 only the modes 'r' and 'rb' will be accepted warning.

I have to say that

folder_node.put_object_from_filelike(io.StringIO('Text'), 'filename.txt', mode='w')

is quite a bit less intuitive than

with self.folder_node.open('filename.txt', 'w') as handle:
    handle.write('Text')

In retrospect it would have been useful to at least point out in the warning how users are supposed to replace existing open calls. Anyhow, I guess now it's too late - the warnings are already gone in develop.

Perhaps the example in this comment can be useful to googling AiiDA users.

[giovannipizzi]

The place to add this note is here: https://github.com/aiidateam/aiida-core/wiki/AiiDA-2.0-plugin-migration-guide
Can you please write a small section there?

[giovannipizzi]

(if you want you can also clarify the docstring:

aiida-core/aiida/orm/nodes/repository.py

Lines 114 to 115 in 7f84b53

	.. note:: this should only be used to open a handle to read an existing file. To write a new file use the method
	``put_object_from_filelike`` instead.

[ltalirz]

The place to add this note is here: https://github.com/aiidateam/aiida-core/wiki/AiiDA-2.0-plugin-migration-guide
Can you please write a small section there?

Thanks, done!