Skip to content

Commit 6d851e6

Browse files
committed
fix: nltk_check() now catches LookupError for missing punkt_tab data
nltk_check() only caught ImportError (package not installed) but missed LookupError (package installed, data not downloaded). Users with nltk present via transitive deps (rouge_score) got an unhelpful ValueError instead of install instructions when punkt_tab was missing. Also adds punkt_tab download to CI workflow — same pattern as Ollama model pulls.
1 parent a6d6758 commit 6d851e6

3 files changed

Lines changed: 20 additions & 4 deletions

File tree

.github/workflows/quality.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ jobs:
4141
- name: Send failure message pre-commit
4242
if: failure() # This step will only run if a previous step failed
4343
run: echo "The quality verification failed. Please run precommit "
44+
- name: Download NLTK data
45+
run: uv run python -m nltk.downloader punkt_tab
4446
- name: Install Ollama
4547
run: curl -fsSL https://ollama.com/install.sh | sh
4648
- name: Start serving ollama

mellea/formatters/granite/base/optional.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ def nltk_check(feature_name: str):
4949
feature_name: Name of the feature that requires NLTK, used in the error message.
5050
5151
Raises:
52-
ImportError: If the ``nltk`` package is not installed, re-raised with
53-
a descriptive message and installation instructions.
52+
ImportError: If the ``nltk`` package is not installed or required
53+
NLTK data (e.g. ``punkt_tab``) has not been downloaded,
54+
re-raised with a descriptive message and installation
55+
instructions.
5456
"""
5557
try:
5658
yield
@@ -60,3 +62,8 @@ def nltk_check(feature_name: str):
6062
f"{feature_name} in the 'granite_io' library."
6163
f"{_NLTK_INSTALL_INSTRUCTIONS}"
6264
) from err
65+
except LookupError as err:
66+
raise ImportError(
67+
f"NLTK data required for {feature_name} is not installed."
68+
f"{_NLTK_INSTALL_INSTRUCTIONS}"
69+
) from err

mellea/formatters/granite/base/util.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ def nltk_check(feature_name: str):
6060
feature_name: Name of the feature that requires NLTK, used in the error message.
6161
6262
Raises:
63-
ImportError: If the ``nltk`` package is not installed, re-raised with
64-
a descriptive message and installation instructions.
63+
ImportError: If the ``nltk`` package is not installed or required
64+
NLTK data (e.g. ``punkt_tab``) has not been downloaded,
65+
re-raised with a descriptive message and installation
66+
instructions.
6567
"""
6668
try:
6769
yield
@@ -71,6 +73,11 @@ def nltk_check(feature_name: str):
7173
f"{feature_name} in the 'granite_io' library."
7274
f"{NLTK_INSTALL_INSTRUCTIONS}"
7375
) from err
76+
except LookupError as err:
77+
raise ImportError(
78+
f"NLTK data required for {feature_name} is not installed."
79+
f"{NLTK_INSTALL_INSTRUCTIONS}"
80+
) from err
7481

7582

7683
def find_substring_in_text(substring: str, text: str) -> list[dict]:

0 commit comments

Comments
 (0)