Skip to content

Commit

Permalink
fix: issue a fatal error when reading an empty mapping
Browse files Browse the repository at this point in the history
Letting an empty mapping get loaded will corrupt langs.json.gz, so
immediately alert the user they cannot do that!

Also, don't override the rules_path if it's already initialized.

Fixes: 322
  • Loading branch information
joanise committed Feb 13, 2024
1 parent 26c4d4a commit b52a819
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 4 deletions.
5 changes: 4 additions & 1 deletion g2p/mappings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,10 @@ def export_to_dict(self, mapping_type="json", config_only=False):
exclude_none=True,
exclude={"parent_dir": True},
)
model_dict["rules_path"] = f"{self.in_lang}_to_{self.out_lang}.{mapping_type}"
if not model_dict.get("rules_path"):
model_dict[
"rules_path"
] = f"{self.in_lang}_to_{self.out_lang}.{mapping_type}"
return model_dict

def config_to_file(
Expand Down
Binary file modified g2p/mappings/langs/langs.json.gz
Binary file not shown.
4 changes: 4 additions & 0 deletions g2p/mappings/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ def load_from_file(path: Union[Path, str]) -> list:
raise exceptions.IncorrectFileType(
f"File {path} is not a valid mapping filetype."
)
if not mapping:
raise exceptions.MalformedMapping(
f"Sorry, the file {path} does not contain any rules."
)
return mapping


Expand Down
1 change: 1 addition & 0 deletions g2p/tests/public/mappings/case-feed/empty.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
t,t,,Actually empty is illegal so create at least one dummy rule
27 changes: 24 additions & 3 deletions g2p/tests/test_z_local_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import yaml

from g2p import exceptions
from g2p.app import APP
from g2p.cli import convert, generate_mapping
from g2p.mappings import Mapping
Expand Down Expand Up @@ -131,14 +132,14 @@ def test_missing_files(self):

with tempfile.TemporaryDirectory() as tmpdir:
config_file = os.path.join(tmpdir, "abbrev-file-not-found.yaml")
with open(os.path.join(tmpdir, "empty.csv"), "wt", encoding="utf8") as f:
pass
with open(os.path.join(tmpdir, "tiny.csv"), "wt", encoding="utf8") as f:
f.write("a,b\n")
with open(config_file, "wt", encoding="utf8") as f:
yaml.dump(
{
"mappings": [
{
"rules_path": "empty.csv",
"rules_path": "tiny.csv",
"abbreviations_path": "no-such-file.csv",
}
]
Expand All @@ -154,6 +155,26 @@ def test_missing_files(self):
results.output + str(results.exception),
)

def test_empty_rules(self):
with tempfile.TemporaryDirectory() as tmpdir:
config_file = os.path.join(tmpdir, "empty-rules-file.yaml")
with open(os.path.join(tmpdir, "empty.csv"), "wt", encoding="utf8") as f:
pass
with open(config_file, "wt", encoding="utf8") as f:
yaml.dump({"mappings": [{"rules_path": "empty.csv"}]}, f)
with self.assertRaises(exceptions.MalformedMapping) as e:
# This is a deep pydantic exception, we should raise MalformedMapping
Mapping.load_mapping_from_path(config_file)
self.assertIn("empty.csv does not contain any rules", str(e.exception))
results = self.runner.invoke(
convert, ["--config", config_file, "a", "b", "c"]
)
self.assertNotEqual(results.exit_code, 0)
self.assertIn(
"empty.csv does not contain any rules",
results.output + str(results.exception),
)

def test_generate_mapping(self):
"""Use a local config to test generate mapping with --from and --to"""
# This test is rather hacky, because it relies on the fact that calling
Expand Down

0 comments on commit b52a819

Please sign in to comment.