Skip to content

Commit

Permalink
Address reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
stuartmcalpine committed Dec 5, 2023
1 parent 7263c51 commit a5932f6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/dataregistry/registrar.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def register_dataset(
Used to associate dataset with a particular execution
access_API : str, optional
Hint as to how to read the data
access_API_configuration : str
access_API_configuration : str, optional
Path to configuration file for `access_API`
is_overwritable : bool, optional
True if dataset may be overwritten (defaults to False).
Expand Down
6 changes: 3 additions & 3 deletions src/dataregistry/registrar_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ def _read_configuration_file(configuration_file, max_config_length):
configuration_file : str
Path to configuration file
max_config_length : int
Maximum number of rows to read. Files above this length will be
truncated.
Maximum number of characters to read from file. Files beyond this limit
will be truncated (with a warning message).
Returns
-------
Expand All @@ -231,7 +231,7 @@ def _read_configuration_file(configuration_file, max_config_length):
if not os.path.isfile(configuration_file):
raise FileNotFoundError(f"{configuration_file} not found")

# Open configuration file and read up to max_config_length lines
# Open configuration file and read up to max_config_length characters
with open(configuration_file) as f:
contents = f.read(max_config_length)

Expand Down
42 changes: 25 additions & 17 deletions tests/unit_tests/test_registrar_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,49 +72,57 @@ def test_directory_info():


def test_name_from_relpath():
"""Make sure names are exctracted from paths correctly"""
"""Make sure names are extracted from paths correctly"""

assert _name_from_relpath("/testing/test") == "test"
assert _name_from_relpath("./testing/test") == "test"
assert _name_from_relpath("/testing/test/") == "test"
assert _name_from_relpath("test") == "test"


@pytest.fixture
def dummy_dir(tmpdir):
"""Make a temp directory to store dummy config files"""
return tmpdir


def _make_dummy_config(tmpdir, nlines):
"""Create a dummy config file in temp directory"""
def _make_dummy_config(tmpdir, nchars):
"""
Create a dummy config file in temp directory
Parameters
----------
tmpdir : py.path.local object (pytest @fixture)
Temporary directory we can store test config files to
nchars : int
Number of characters to put in temp config file
Returns
-------
file_path : str
Path to temporary config file we can read
"""

file_path = os.path.join(tmpdir, "dummy_config.txt")

# Write nlines into the dummy file
# Write nchars characters into the dummy file
with open(file_path, "w") as file:
for i in range(nlines):
file.write(f"I am line {i}\n")
for i in range(nchars):
file.write(f"X")

return file_path


@pytest.mark.parametrize("nlines,max_config_length,ans", [(10, 10, 10), (100, 10, 10)])
def test_read_file(dummy_dir, nlines, max_config_length, ans):
@pytest.mark.parametrize("nchars,max_config_length,ans", [(10, 10, 10), (100, 10, 10)])
def test_read_file(tmpdir, nchars, max_config_length, ans):
"""Test reading in configuration file, and check truncation warning"""

# Make sure we warn when truncating
if nlines > max_config_length:
if nchars > max_config_length:
with pytest.warns(UserWarning, match="Configuration file is longer"):
content = _read_configuration_file(
_make_dummy_config(dummy_dir, nlines), max_config_length
_make_dummy_config(tmpdir, nchars), max_config_length
)
assert len(content) == ans

# Usual case
else:
content = _read_configuration_file(
_make_dummy_config(dummy_dir, nlines), max_config_length
_make_dummy_config(tmpdir, nchars), max_config_length
)
assert len(content) == ans

Expand Down

0 comments on commit a5932f6

Please sign in to comment.