diff --git a/CHANGELOG.md b/CHANGELOG.md index bacf863a..4bfd0186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Release Notes +## Unreleased + +### Added +- Stricter validation of input hypotheses and references + ## 2.6.0 (2026-01-12) - Dropped Python 3.8, added Python 3.13 (requires-python = ">=3.9") - License Format: Changed to PEP 639 bare SPDX format (license = "Apache-2.0") diff --git a/sacrebleu/metrics/base.py b/sacrebleu/metrics/base.py index b0141247..5315eb4d 100644 --- a/sacrebleu/metrics/base.py +++ b/sacrebleu/metrics/base.py @@ -226,9 +226,11 @@ def _check_sentence_score_args(self, hyp: str, refs: Sequence[str]): if not isinstance(hyp, str): err_msg = "The argument `hyp` should be a string." - elif isinstance(refs, str) or not isinstance(refs, Sequence): + elif isinstance(refs, (str, bytes)) or not isinstance(refs, Sequence): err_msg = "The argument `refs` should be a sequence of strings." - elif not isinstance(refs[0], str) and refs[0] is not None: + elif len(refs) == 0: + err_msg = "The argument `refs` should not be empty." + elif any(not isinstance(ref, str) and ref is not None for ref in refs): err_msg = "Each element of `refs` should be a string." if err_msg: @@ -239,7 +241,7 @@ def _check_corpus_score_args( ): """Performs sanity checks on `corpus_score` method's arguments. - :param hypses: A sequence of hypothesis strings. + :param hyps: A sequence of hypothesis strings. :param refs: A sequence of reference documents with document being defined as a sequence of reference strings. If `None`, cached references will be used. @@ -248,20 +250,40 @@ def _check_corpus_score_args( prefix = self.__class__.__name__ err_msg = None - if not isinstance(hyps, Sequence): + if isinstance(hyps, (str, bytes)) or not isinstance(hyps, Sequence): err_msg = "`hyps` should be a sequence of strings." - elif not isinstance(hyps[0], str): - err_msg = "Each element of `hyps` should be a string." + elif len(hyps) == 0: + err_msg = "`hyps` should not be empty." elif any(line is None for line in hyps): err_msg = "Undefined line in hypotheses stream!" + elif any(not isinstance(hyp, str) for hyp in hyps): + err_msg = "Each element of `hyps` should be a string." - if refs is not None: + if refs is not None and err_msg is None: if not isinstance(refs, Sequence): err_msg = "`refs` should be a sequence of sequence of strings." - elif not isinstance(refs[0], Sequence): - err_msg = "Each element of `refs` should be a sequence of strings." - elif not isinstance(refs[0][0], str) and refs[0][0] is not None: - err_msg = "`refs` should be a sequence of sequence of strings." + elif len(refs) == 0: + err_msg = "`refs` should not be empty." + else: + for refs_streams in refs: + if isinstance(refs_streams, (str, bytes)) or not isinstance( + refs_streams, Sequence + ): + err_msg = ( + "Each element of `refs` should be a sequence of strings." + ) + elif len(refs_streams) != len(hyps): + err_msg = ( + "Each references stream in `refs` must have the same " + "length as `hyps`." + ) + elif any( + not isinstance(line, str) and line is not None + for line in refs_streams + ): + err_msg = "`refs` should be a sequence of sequence of strings." + if err_msg is not None: + break if err_msg: raise TypeError(f"{prefix}: {err_msg}") diff --git a/test/test_metrics_base.py b/test/test_metrics_base.py new file mode 100644 index 00000000..679ec714 --- /dev/null +++ b/test/test_metrics_base.py @@ -0,0 +1,355 @@ +from contextlib import nullcontext as does_not_raise + +import pytest + +from sacrebleu.metrics.base import Metric + + +class ConcreteTestMetric(Metric): + """Concrete test metric so the ABC can be instantiated.""" + + def _aggregate_and_compute(self, *args, **kwargs): ... + + def _compute_score_from_stats(self, *args, **kwargs): ... + + def _preprocess_segment(self, *args, **kwargs): ... + + def _extract_reference_info(self, *args, **kwargs): ... + + def _compute_segment_statistics(self, *args, **kwargs): ... + + +@pytest.mark.parametrize( + "hyp, refs, expected_context", + [ + # valid + pytest.param( + "h1", + ["r1"], + does_not_raise(), + id="valid-minimal", + ), + pytest.param( + "h1", + ["r1", "r2"], + does_not_raise(), + id="valid-multi-reference", + ), + pytest.param( + "h1", + ("r1",), + does_not_raise(), + id="valid-tuple", + ), + pytest.param( + "", + ["r1"], + does_not_raise(), + id="valid-empty-hyp-string", + ), + pytest.param( + "h1", + ["r1", None], + does_not_raise(), + id="valid-none-reference-element", + ), + pytest.param( + "h1", + [None], + does_not_raise(), + id="valid-none-only-reference", + ), + # invalid hyp + pytest.param( + 123, + ["r1"], + pytest.raises(TypeError, match="The argument `hyp` should be a string"), + id="hyp-int", + ), + pytest.param( + None, + ["r1"], + pytest.raises(TypeError, match="The argument `hyp` should be a string"), + id="hyp-none", + ), + pytest.param( + ["h1"], + ["r1"], + pytest.raises(TypeError, match="The argument `hyp` should be a string"), + id="hyp-is-sequence", + ), + # invalid refs + pytest.param( + "h1", + "r1", + pytest.raises( + TypeError, match="The argument `refs` should be a sequence of strings" + ), + id="refs-bare-string", + ), + pytest.param( + "h1", + b"r1", + pytest.raises( + TypeError, match="The argument `refs` should be a sequence of strings" + ), + id="refs-bytes", + ), + pytest.param( + "h1", + 123, + pytest.raises( + TypeError, match="The argument `refs` should be a sequence of strings" + ), + id="refs-not-sequence", + ), + pytest.param( + "h1", + [], + pytest.raises(TypeError, match="The argument `refs` should not be empty"), + id="refs-empty", + ), + pytest.param( + "h1", + [123], + pytest.raises(TypeError, match="Each element of `refs` should be a string"), + id="refs-element-not-string", + ), + pytest.param( + "h1", + ["r1", 5], + pytest.raises(TypeError, match="Each element of `refs` should be a string"), + id="refs-bad-later-element", + ), + # both invalid + pytest.param( + 123, + "r1", + pytest.raises(TypeError, match="The argument `hyp` should be a string"), + id="both-invalid-hyp-message-wins", + ), + ], +) +def test_check_sentence_score_args(hyp, refs, expected_context): + metric = ConcreteTestMetric() + with expected_context: + metric._check_sentence_score_args(hyp=hyp, refs=refs) + + +@pytest.mark.parametrize( + "hyps, refs, expected_context", + [ + # valid hyps and refs + pytest.param( + ["h1"], + [["r1a"]], + does_not_raise(), + id="valid-minimal", + ), + pytest.param( + ("h1",), + (("r1a",),), + does_not_raise(), + id="valid-tuples", + ), + pytest.param( + ["h1"], + [["r1a"], ["r2"]], + does_not_raise(), + id="valid-multi-reference-single-segment", + ), + pytest.param( + ["h1", "h2"], + [["r1a", "r2a"], ["r1b", "r2b"]], + does_not_raise(), + id="valid-multi-stream-equal-length", + ), + pytest.param( + ["h1", "h2"], + [["r1a", "r2a"], ["r1b", None]], + does_not_raise(), + id="valid-none-padding-equal-length", + ), + pytest.param(["h1"], None, does_not_raise(), id="valid-refs-none"), + pytest.param( + ["h1"], [[None]], does_not_raise(), id="valid-none-reference-element" + ), + # invalid hyps + pytest.param( + "h1", + None, + pytest.raises(TypeError, match="`hyps` should be a sequence of strings"), + id="hyps-bare-string", + ), + pytest.param( + b"h1", + None, + pytest.raises(TypeError, match="`hyps` should be a sequence of strings"), + id="hyps-bare-bytes", + ), + pytest.param( + 123, + None, + pytest.raises(TypeError, match="`hyps` should be a sequence of strings"), + id="hyps-int", + ), + pytest.param( + None, + None, + pytest.raises(TypeError, match="`hyps` should be a sequence of strings"), + id="hyps-none", + ), + pytest.param( + [], + None, + pytest.raises(TypeError, match="`hyps` should not be empty"), + id="hyps-empty", + ), + pytest.param( + [123], + None, + pytest.raises(TypeError, match="Each element of `hyps` should be a string"), + id="hyps-non-string-element", + ), + pytest.param( + ["hyp1", 123], + None, + pytest.raises(TypeError, match="Each element of `hyps` should be a string"), + id="hyps-non-string-later-element", + ), + pytest.param( + [None], + None, + pytest.raises(TypeError, match="Undefined line in hypotheses stream"), + id="hyps-first-element-none", + ), + pytest.param( + ["h1", None], + None, + pytest.raises(TypeError, match="Undefined line in hypotheses stream"), + id="hyps-trailing-none", + ), + # invalid refs - types + pytest.param( + ["h1"], + "r1a", + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-bare-string", + ), + pytest.param( + ["h1"], + b"r1", + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-bare-bytes", + ), + pytest.param( + ["h1"], + 123, + pytest.raises( + TypeError, match="`refs` should be a sequence of sequence of strings" + ), + id="refs-int", + ), + pytest.param( + ["h1"], + [], + pytest.raises(TypeError, match="`refs` should not be empty"), + id="refs-empty", + ), + pytest.param( + ["h1"], + ["r1a"], + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-flat-list-of-strings", + ), + pytest.param( + ["h1"], + [123], + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-element-not-sequence", + ), + pytest.param( + ["h1"], + [[123]], + pytest.raises( + TypeError, match="`refs` should be a sequence of sequence of strings" + ), + id="refs-inner-element-not-string", + ), + # invalid refs - lengths + pytest.param( + ["h1", "h2", "h3"], + [["r1a", "r2a"]], + pytest.raises(TypeError, match="same length as `hyps`"), + id="refs-stream-too-short", + ), + pytest.param( + ["h1", "h2"], + [["r1a", "r2a", "r3a"]], + pytest.raises(TypeError, match="same length as `hyps`"), + id="refs-stream-too-long", + ), + pytest.param( + ["h1", "h2"], + [["r1a", "r2a"], ["r1b"]], + pytest.raises(TypeError, match="same length as `hyps`"), + id="refs-streams-unequal", + ), + pytest.param( + ["h1"], + [[]], + pytest.raises(TypeError, match="same length as `hyps`"), + id="refs-empty-inner-stream", + ), + # per-stream validation + pytest.param( + ["h1", "h2"], + [["r1a", "r2a"], "bad"], + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-later-stream-bare-string", + ), + pytest.param( + ["h1"], + [["r1a"], 5], + pytest.raises( + TypeError, + match="Each element of `refs` should be a sequence of strings", + ), + id="refs-later-sequence-not-sequence", + ), + pytest.param( + ["h1", "h2"], + [["r1a", "r2a"], ["r1b", 99]], + pytest.raises( + TypeError, match="`refs` should be a sequence of sequence of strings" + ), + id="refs-later-stream-non-string-element", + ), + # both invalid + pytest.param( + "h1", + "r1a", + pytest.raises(TypeError, match="`hyps` should be a sequence of strings"), + id="both-invalid-hyps-message-wins", + ), + ], +) +def test_check_corpus_score_args(hyps, refs, expected_context): + metric = ConcreteTestMetric() + with expected_context: + metric._check_corpus_score_args(hyps=hyps, refs=refs)