Skip to content

Commit

Permalink
SinglefileData: Fix bug when filename is pathlib.Path (#6006)
Browse files Browse the repository at this point in the history
It would raise an exception when the node was stored since it would
compare the `filename` against the list of objects, but the latter would
be instances of a string instead of `pathlib.Path` and the validation
would fail. On top of that, the difference in type was not visible as
the cause in the error message, making for a cryptic error.
  • Loading branch information
sphuber committed May 11, 2023
1 parent 685e0f8 commit f36bf58
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
11 changes: 6 additions & 5 deletions aiida/orm/nodes/data/singlefile.py
Expand Up @@ -8,6 +8,8 @@
# For further information please visit http://www.aiida.net #
###########################################################################
"""Data class that can be used to store a single file in its repository."""
from __future__ import annotations

import contextlib
import os
import pathlib
Expand All @@ -24,7 +26,7 @@ class SinglefileData(Data):

DEFAULT_FILENAME = 'file.txt'

def __init__(self, file, filename=None, **kwargs):
def __init__(self, file, filename: str | pathlib.Path | None = None, **kwargs):
"""Construct a new instance and set the contents to that of the file.
:param file: an absolute filepath or filelike object whose contents to copy.
Expand Down Expand Up @@ -67,7 +69,7 @@ def get_content(self):
with self.open() as handle:
return handle.read()

def set_file(self, file, filename=None):
def set_file(self, file, filename: str | pathlib.Path | None = None):
"""Store the content of the file in the node's repository, deleting any other existing objects.
:param file: an absolute filepath or filelike object whose contents to copy
Expand All @@ -92,8 +94,7 @@ def set_file(self, file, filename=None):
except AttributeError:
key = self.DEFAULT_FILENAME

key = filename or key

key = str(filename) if filename is not None else key
existing_object_names = self.base.repository.list_object_names()

try:
Expand Down Expand Up @@ -126,5 +127,5 @@ def _validate(self):

if [filename] != objects:
raise exceptions.ValidationError(
f'respository files {objects} do not match the `filename` attribute {filename}.'
f'respository files {objects} do not match the `filename` attribute `{filename}`.'
)
10 changes: 5 additions & 5 deletions tests/orm/nodes/data/test_singlefile.py
Expand Up @@ -32,7 +32,7 @@ def inner(node, content_reference, filename, open_mode='r'):
with node.open(mode=open_mode) as handle:
assert handle.read() == content_reference

assert node.base.repository.list_object_names() == [filename]
assert node.base.repository.list_object_names() == [str(filename)]

return inner

Expand Down Expand Up @@ -145,16 +145,16 @@ def test_construct_with_path(check_singlefile_content_with_store):
)


def test_construct_with_filename(check_singlefile_content):
@pytest.mark.parametrize('filename', ('myfile.txt', pathlib.Path('myfile.txt')))
def test_construct_with_filename(check_singlefile_content_with_store, filename):
"""Test constructing an instance, providing a filename."""
content_original = 'some testing text\nwith a newline'
filename = 'myfile.txt'

# test creating from string
with io.BytesIO(content_original.encode('utf-8')) as handle:
node = SinglefileData(file=handle, filename=filename)

check_singlefile_content(node=node, content_reference=content_original, filename=filename)
check_singlefile_content_with_store(node=node, content_reference=content_original, filename=filename)

# test creating from file
with tempfile.NamedTemporaryFile(mode='wb+') as handle:
Expand All @@ -163,7 +163,7 @@ def test_construct_with_filename(check_singlefile_content):
handle.seek(0)
node = SinglefileData(file=handle, filename=filename)

check_singlefile_content(node=node, content_reference=content_original, filename=filename)
check_singlefile_content_with_store(node=node, content_reference=content_original, filename=filename)


def test_binary_file(check_singlefile_content_with_store):
Expand Down

0 comments on commit f36bf58

Please sign in to comment.